Bug 174445

Summary: Add WebDriver support in browser driver part of BenchmarkRunner
Product: WebKit Reporter: Matthew Stewart <matthew_r_stewart>
Component: New BugsAssignee: 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 Flags
Patch
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch
none
Patch
none
Patch none

Matthew Stewart
Reported 2017-07-12 17:22:03 PDT
Add WebDriver support in browser driver part of BenchmarkRunner
Attachments
Patch (17.49 KB, patch)
2017-07-12 17:36 PDT, Matthew Stewart 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (11.35 MB, application/zip)
2017-07-12 19:40 PDT, Build Bot 		no flags
Details
Patch (17.92 KB, patch)
2017-07-17 15:02 PDT, Matthew Stewart 		no flags
DetailsFormatted DiffDiff
Patch (18.08 KB, patch)
2017-07-17 16:37 PDT, Matthew Stewart 		no flags
DetailsFormatted DiffDiff
Patch (18.15 KB, patch)
2017-07-19 13:56 PDT, Matthew Stewart 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Matthew Stewart
Comment 1 2017-07-12 17:36:23 PDT
Created attachment 315317 [details] Patch
Stephanie Lewis
Comment 2 2017-07-12 17:58:00 PDT
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.
Build Bot
Comment 3 2017-07-12 19:40:53 PDT
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
Build Bot
Comment 4 2017-07-12 19:40:54 PDT
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
Matthew Stewart
Comment 5 2017-07-17 15:02:24 PDT
Created attachment 315719 [details] Patch
Stephanie Lewis
Comment 6 2017-07-17 15:34:05 PDT
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.
Matthew Stewart
Comment 7 2017-07-17 16:37:27 PDT
Created attachment 315726 [details] Patch
dewei_zhu
Comment 8 2017-07-18 14:21:37 PDT
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.
Matthew Stewart
Comment 9 2017-07-19 13:56:29 PDT
Created attachment 315955 [details] Patch
dewei_zhu
Comment 10 2017-07-19 17:26:37 PDT
Comment on attachment 315955 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315955&action=review > Tools/ChangeLog:5 > + r=me
WebKit Commit Bot
Comment 11 2017-07-24 17:40:55 PDT
Comment on attachment 315955 [details] Patch Clearing flags on attachment: 315955 Committed r219850: <http://trac.webkit.org/changeset/219850>
WebKit Commit Bot
Comment 12 2017-07-24 17:40:57 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.