Changeset 220246 in webkit

Timestamp:

Aug 3, 2017 6:12:52 PM (7 years ago)

Author:

clopez@igalia.com

Message:

REGRESSION(r219850): run-benchmark script broken on Linux
​https://bugs.webkit.org/show_bug.cgi?id=175126

Reviewed by Stephanie Lewis.

The run-benchmark script dynamically generates the list of supported
browsers and platforms (currently Linux and OSX) by loading all
python files from Tools/Scripts/webkitpy/benchmark_runner/browser_driver
and getting the browser_name and platform variables from the
classes defined there.

This means that this classes should not raise an exception when
loaded on other platforms or otherwise they will broke the whole
script. Its fine if they raise an exception when executing any of
the methods they implement, but not when just loading/importing
the class.

Move the argument variable definitions that call on the platform
specific OSXBrowserDriver._screen_size() function from beeing
variables that are evaluated when loading the file, to be functions
that are only evaluated when the actual functionality needs to be
executed.

• Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:

(OSXChromeDriver.launch_url):
(OSXChromeCanaryDriver.launch_url):
(create_args):
(create_chrome_options):
(create_window_size_arg):

• Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py:

(OSXFirefoxDriver.launch_url):
(OSXFirefoxNightlyDriver.launch_url):
(OSXFirefoxNightlyDriver.launch_driver):
(create_args):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py (modified) (5 diffs)
• Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py (modified) (4 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2017-08-03  Carlos Alberto Lopez Perez  <clopez@igalia.com>
+
+        REGRESSION(r219850): run-benchmark script broken on Linux
+        https://bugs.webkit.org/show_bug.cgi?id=175126
+
+        Reviewed by Stephanie Lewis.
+
+        The run-benchmark script dynamically generates the list of supported
+        browsers and platforms (currently Linux and OSX) by loading all
+        python files from Tools/Scripts/webkitpy/benchmark_runner/browser_driver
+        and getting the browser_name and platform variables from the
+        classes defined there.
+
+        This means that this classes should not raise an exception when
+        loaded on other platforms or otherwise they will broke the whole
+        script. Its fine if they raise an exception when executing any of
+        the methods they implement, but not when just loading/importing
+        the class.
+
+        Move the argument variable definitions that call on the platform
+        specific OSXBrowserDriver._screen_size() function from beeing
+        variables that are evaluated when loading the file, to be functions
+        that are only evaluated when the actual functionality needs to be
+        executed.
+
+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:
+        (OSXChromeDriver.launch_url):
+        (OSXChromeCanaryDriver.launch_url):
+        (create_args):
+        (create_chrome_options):
+        (create_window_size_arg):
+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py:
+        (OSXFirefoxDriver.launch_url):
+        (OSXFirefoxNightlyDriver.launch_url):
+        (OSXFirefoxNightlyDriver.launch_driver):
+        (create_args):
+
 2017-08-03  Yoshiaki Jitsukawa  <jitsu@rd.scei.sony.co.jp>
 

trunk/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py

@@ -8 +8 @@
 
 _log = logging.getLogger(__name__)
-window_size_arg = '--window-size={width},{height}'.format(width=int(OSXBrowserDriver._screen_size().width), height=int(OSXBrowserDriver._screen_size().height))
-args = ['--args', '--homepage', window_size_arg]
 
 class OSXChromeDriver(OSXBrowserDriver):
@@ -17 +15 @@
 
     def launch_url(self, url, options, browser_build_path):
-        args_with_url = self._insert_url(args, 2, url)
+        args_with_url = self._insert_url(create_args(), 2, url)
         self._launch_process(build_dir=browser_build_path, app_name=self.app_name, url=url, args=args_with_url)
 
@@ -39 +37 @@
 
     def launch_url(self, url, options, browser_build_path):
-        args_with_url = self._insert_url(args, 2, url)
+        args_with_url = self._insert_url(create_args(), 2, url)
         self._launch_process(build_dir=browser_build_path, app_name=self.app_name, url=url, args=args_with_url)
 
@@ -56 +54 @@
 
 
+def create_args():
+    args = ['--args', '--homepage', create_window_size_arg()]
+    return args
+
 def create_chrome_options():
     from webkitpy.thirdparty.autoinstalled.selenium.webdriver.chrome.options import Options
@@ -62 +64 @@
     chrome_options.add_argument("--user-data-dir")
     chrome_options.add_argument("--disable-extensions")
-    chrome_options.add_argument(window_size_arg)
+    chrome_options.add_argument(create_window_size_arg())
     return chrome_options
+
+
+def create_window_size_arg():
+    window_size_arg = '--window-size={width},{height}'.format(width=int(OSXBrowserDriver._screen_size().width), height=int(OSXBrowserDriver._screen_size().height))
+    return window_size_arg

trunk/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py

@@ -9 +9 @@
 _log = logging.getLogger(__name__)
 
-args = ['--args', '-width', str(int(OSXBrowserDriver._screen_size().width)), '-height', str(int(OSXBrowserDriver._screen_size().height))]
-
 
 class OSXFirefoxDriver(OSXBrowserDriver):
@@ -18 +16 @@
 
     def launch_url(self, url, options, browser_build_path):
-        args_with_url = self._insert_url(args, 0, url)
+        args_with_url = self._insert_url(create_args(), 0, url)
         self._launch_process(build_dir=browser_build_path, app_name=self.app_name, url=url, args=args_with_url)
 
@@ -41 +39 @@
 
     def launch_url(self, url, options, browser_build_path):
-        args_with_url = self._insert_url(args, 0, url)
+        args_with_url = self._insert_url(create_args(), 0, url)
         self._launch_process(build_dir=browser_build_path, app_name=self.app_name, url=url, args=args_with_url)
 
@@ -57 +55 @@
         self._launch_webdriver(url=url, driver=driver)
         return driver
+
+
+def create_args():
+    args = ['--args', '-width', str(int(OSXBrowserDriver._screen_size().width)), '-height', str(int(OSXBrowserDriver._screen_size().height))]
+    return args