Bug 174056 - WIP - RunBenchmark WebDriver testing
Summary: WIP - RunBenchmark WebDriver testing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 174331 174390 174443 174445
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-30 16:57 PDT by Matthew Stewart
Modified: 2017-09-08 15:25 PDT (History)
6 users (show)

See Also:


Attachments
Patch (869.90 KB, patch)
2017-06-30 17:02 PDT, Matthew Stewart
slewis: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Stewart 2017-06-30 16:57:55 PDT
WIP - RunBenchmark WebDriver testing
Comment 1 Matthew Stewart 2017-06-30 17:02:20 PDT
Created attachment 314327 [details]
Patch
Comment 2 Build Bot 2017-06-30 17:05:50 PDT
Attachment 314327 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_safari_driver.py:36:  [OSXSafariDriver.close_browsers] Undefined variable 'sys'  [pylint/E0602] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_safari_driver.py:36:  [OSXSafariDriver.close_browsers] Instance of 'OSXSafariDriver' has no '_process' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/utils.py:51:  [write_defaults] No name 'NSUserDefaults' in module 'Foundation'  [pylint/E0611] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/linux_browser_driver.py:92:  [LinuxBrowserDriver._launch_driver] Method should have "self" as first argument  [pylint/E0213] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_chrome_driver.py:19:  missing whitespace around operator  [pep8/E225] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_chrome_driver.py:35:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_chrome_driver.py:38:  missing whitespace around operator  [pep8/E225] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_firefox_driver.py:19:  missing whitespace around operator  [pep8/E225] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_firefox_driver.py:31:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_firefox_driver.py:34:  missing whitespace around operator  [pep8/E225] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_browser_driver.py:20:  [OSXBrowserDriver.prepare_env] No name 'CGWarpMouseCursorPosition' in module 'Quartz'  [pylint/E0611] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_browser_driver.py:46:  [OSXBrowserDriver._screen_size] No name 'NSScreen' in module 'AppKit'  [pylint/E0611] [5]
Total errors found: 12 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 dewei_zhu 2017-06-30 17:58:19 PDT
Comment on attachment 314327 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314327&action=review

> Tools/ChangeLog:7
> +

A lot of files are duplicated. We should try to figure out a way to reuse the code from run-benchmark.

> Tools/Scripts/webkitpy/webdriver_benchmark_runner/README.md:1
> +# Benchmark Runner 

Looks like this file need to be updated.

> Tools/Scripts/webkitpy/webdriver_benchmark_runner/benchmark_runner.py:19
> +'''from selenium import webdriver
> +from selenium.webdriver.chrome.options import Options'''

We should remove commented code.

> Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/linux_browser_driver.py:32
> +from selenium import webdriver

Is this used?
Comment 4 Brian Burg 2017-07-03 10:44:08 PDT
Comment on attachment 314327 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314327&action=review

> Tools/Scripts/webkitpy/webdriver_benchmark_runner/benchmark_runner.py:74
> +            url = 'file://' + web_root + '/' + self._plan_name + '/' + test_file

This would look a lot better using a template/format string.

> Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_safari_driver.py:30
> +        self._launch_driver(build_dir=browser_build_path, app_name="Safari.app", url=url, args=["-HomePage", "about:blank", "-WarnAboutFraudulentWebsites", "0", "-ExtensionsEnabled", "0", "-ShowStatusBar", "0", "-NewWindowBehavior", "1", "-NewTabBehavior", "1"], driver=driver)

If these are needed to avoid hangs while running WebDriver tests, you should file a bug against safaridriver. Hangs should never happen during a WebDriver test.

> Tools/Scripts/webkitpy/webdriver_benchmark_runner/browser_driver/osx_safari_driver.py:39
> +    def _maximize_window(cls):

You can do this via WebDriver API. Why is this needed?