Merge lp://qastaging/~vila/selenium-simple-test/1161336-hookable-browser into lp://qastaging/selenium-simple-test
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Corey Goldberg | ||||
Approved revision: | 373 | ||||
Merged at revision: | 375 | ||||
Proposed branch: | lp://qastaging/~vila/selenium-simple-test/1161336-hookable-browser | ||||
Merge into: | lp://qastaging/selenium-simple-test | ||||
Diff against target: |
1298 lines (+315/-296) 12 files modified
src/sst/actions.py (+66/-182) src/sst/bmobproxy.py (+1/-0) src/sst/command.py (+13/-10) src/sst/context.py (+1/-3) src/sst/runtests.py (+175/-55) src/sst/scripts/remote.py (+13/-8) src/sst/scripts/run.py (+12/-5) src/sst/selftests/test_two_methods.py (+0/-1) src/sst/tests/test_runtests_get_suites.py (+10/-19) src/sst/tests/test_sst_run.py (+0/-4) src/sst/tests/test_sst_script_test_case.py (+23/-8) src/sst/tests/test_sst_test_case.py (+1/-1) |
||||
To merge this branch: | bzr merge lp://qastaging/~vila/selenium-simple-test/1161336-hookable-browser | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Corey Goldberg (community) | Approve | ||
Review via email:
|
Commit message
Make the selenium webdriver object a test attribute allowing tests to override it to suit their needs
Description of the change
This fixes bug #1161336 by making the selenium webdriver object a test attribute and as such remove the global variables 'browser' and 'browsermob_proxy' used in actions.py.
The new design introduces browser factories classes that can create the right type of browser.
Since some browser properties can be set by the tests themselves, the browser creation must be delayed until the test setup. Yet, some of these options can also be provided from the command line so there was a need to collect them, hence the factory.
The BrowserFactory can collect browser properties (is javascript enabled or the like) so they can be provided to the test setup.
This design simplify the functions used to create the tests (runtests, get_suites, get_suite, get_case) from the scripts since they can carry only the browser factory instead of all browser options.
Additionnally, this allowed some more simplification as some options are browser specific (most notably, the ones required by sst-remote doesn't have to be carried when using sst-run and vice-versa).
For lack of a better way, the browser factory is also responsible for handling the browsermob_proxy (missing tests). This allowed removing the 'browsermob_proxy' global variable too.
From there, a few modifications had to be made in actions.py:
- all access to the browser objects are now done via the '_test' global variable which each test set during its setup, this allowed removing the 'browser' global variable,
- the actions 'start' and 'stop' have been removed (their core is now part of the test classes). Since sst guarantee that the browser is handled automatically, there is no point in allowing these actions to a user,
- the http capture features have been factorized and moved to the browser_factory object (may be a bit controversial but given their implementation that was the best fit),
Overall, this refactoring could have been simpler if we had a better test
coverage. Namely, the following areas were not covered:
* no tests for sst-remote (I'm unclear about which feature is provided here and how to set it up),
* there are a bunch of _make_results_dir calls in actions.py which shouldn't be necessary, _make_results_dir is already called during test setUp() (unless that one is wrong ?),
* no tests for .har files (would probably help remove any reamining controversy around where the methods should be defined ;)
Final note, I merged the test from https:/
I like this.
I'd really like to remove browsermob integration feature in it's entirety as it would much simplify things. (including this new design)... will do so in future branch so this doesn't block progress on other projects.