| Summary: | Add WebDriver support in browser driver part of BenchmarkRunner | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Matthew Stewart <matthew_r_stewart> | ||||||||||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | buildbot, commit-queue, dewei_zhu, glenn, slewis | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | 175126 | ||||||||||||||
| Bug Blocks: | 174056, 174331 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Matthew Stewart
2017-07-12 17:22:03 PDT
Created attachment 315317 [details]
Patch
Comment on attachment 315317 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315317&action=review > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:29 > + args.insert(2, url) This feels clunky. Given this code is duplicated below I think it worth having helper function just to give it a name like insert_url_into_arguments. Also rather than changing the args array itself I'd make a new array so we don't have to worry about the constants being in a bad state. > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py:25 > + args.insert(0, url) ditto. Your insert function should probably take a pos and return the new args array and can be used in both places. Comment on attachment 315317 [details] Patch Attachment 315317 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4110917 New failing tests: imported/w3c/IndexedDB-private-browsing/idbfactory_open12.html Created attachment 315328 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Created attachment 315719 [details]
Patch
Comment on attachment 315719 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315719&action=review Otherwise patch looks awesome! > Tools/ChangeLog:8 > + * Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py: This changelog is long enough that you should have a paragraph under the reviewed bit describing in more detail what it does. Also some minimal annotation on the various functions to describe their purpose can be useful. Created attachment 315726 [details]
Patch
Comment on attachment 315726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315726&action=review > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:22 > + def launch_driver(self, url, driver): > + pass This function name is ambiguous with 'launch_driver(self, url, options, browser_build_path)'. > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_minibrowsergtk_driver.py:43 > + raise ValueError("Browser \"%s\" is not available with webdriver" % self.browser_name) str.format is preferred. Created attachment 315955 [details]
Patch
Comment on attachment 315955 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315955&action=review > Tools/ChangeLog:5 > + r=me Comment on attachment 315955 [details] Patch Clearing flags on attachment: 315955 Committed r219850: <http://trac.webkit.org/changeset/219850> All reviewed patches have been landed. Closing bug. |