Bug 1338397 - Use timeout multipler to adjust browser startup timeout in wpt, r=maja_zf draft
authorJames Graham <james@hoppipolla.co.uk>
Thu, 23 Mar 2017 17:50:19 +0000
changeset 552506 592fc93705a1fa36fcd2e674141f514fcc781117
parent 552445 d4af7ec6cfcd9b81cd1f433a00b412de61e95b62
child 552532 7845633899201b75c7fe69a18597690d1ed8bef4
push id51362
push userbmo:james@hoppipolla.co.uk
push dateTue, 28 Mar 2017 14:40:19 +0000
reviewersmaja_zf
bugs1338397
milestone55.0a1
Bug 1338397 - Use timeout multipler to adjust browser startup timeout in wpt, r=maja_zf For gecko builds with --disable-optimize everything is very very slow; enough that the fixed 60s timeout for the browser to start is insufficient, and various marionette timeouts may also be hit. To alleviate this problem either disable timeouts or multiply them by the timeout multiplier which is generally set to around 3 for debug builds. This seems sufficient to solve the problem on the hardware developers actually have. MozReview-Commit-ID: I3zHJGXlpnd
testing/web-platform/harness/wptrunner/browsers/chrome.py
testing/web-platform/harness/wptrunner/browsers/edge.py
testing/web-platform/harness/wptrunner/browsers/firefox.py
testing/web-platform/harness/wptrunner/browsers/servo.py
testing/web-platform/harness/wptrunner/browsers/servodriver.py
testing/web-platform/harness/wptrunner/executors/executormarionette.py
testing/web-platform/harness/wptrunner/wptrunner.py
--- a/testing/web-platform/harness/wptrunner/browsers/chrome.py
+++ b/testing/web-platform/harness/wptrunner/browsers/chrome.py
@@ -18,17 +18,17 @@ from ..executors.executorselenium import
                  "executor_kwargs": "executor_kwargs",
                  "env_options": "env_options"}
 
 
 def check_args(**kwargs):
     require_arg(kwargs, "webdriver_binary")
 
 
-def browser_kwargs(**kwargs):
+def browser_kwargs(test_type, run_info_data, **kwargs):
     return {"binary": kwargs["binary"],
             "webdriver_binary": kwargs["webdriver_binary"]}
 
 
 def executor_kwargs(test_type, server_config, cache_manager, run_info_data,
                     **kwargs):
     from selenium.webdriver import DesiredCapabilities
 
--- a/testing/web-platform/harness/wptrunner/browsers/edge.py
+++ b/testing/web-platform/harness/wptrunner/browsers/edge.py
@@ -16,17 +16,17 @@ from ..executors.executorselenium import
                  "browser_kwargs": "browser_kwargs",
                  "executor_kwargs": "executor_kwargs",
                  "env_options": "env_options"}
 
 
 def check_args(**kwargs):
     require_arg(kwargs, "webdriver_binary")
 
-def browser_kwargs(**kwargs):
+def browser_kwargs(test_type, run_info_data, **kwargs):
     return {"webdriver_binary": kwargs["webdriver_binary"]}
 
 def executor_kwargs(test_type, server_config, cache_manager, run_info_data,
                     **kwargs):
     from selenium.webdriver import DesiredCapabilities
 
     executor_kwargs = base_executor_kwargs(test_type, server_config,
                                            cache_manager, **kwargs)
--- a/testing/web-platform/harness/wptrunner/browsers/firefox.py
+++ b/testing/web-platform/harness/wptrunner/browsers/firefox.py
@@ -39,49 +39,60 @@ here = os.path.join(os.path.split(__file
                               "wdspec": "MarionetteWdspecExecutor"},
                  "browser_kwargs": "browser_kwargs",
                  "executor_kwargs": "executor_kwargs",
                  "env_options": "env_options",
                  "run_info_extras": "run_info_extras",
                  "update_properties": "update_properties"}
 
 
+def get_timeout_multiplier(test_type, run_info_data, **kwargs):
+    if kwargs["timeout_multiplier"] is not None:
+        return kwargs["timeout_multiplier"]
+    if test_type == "reftest":
+        if run_info_data["debug"] or run_info_data.get("asan"):
+            return 4
+        else:
+            return 2
+    elif run_info_data["debug"] or run_info_data.get("asan"):
+        return 3
+    return 1
+
+
 def check_args(**kwargs):
     require_arg(kwargs, "binary")
     if kwargs["ssl_type"] != "none":
         require_arg(kwargs, "certutil_binary")
 
 
-def browser_kwargs(**kwargs):
+def browser_kwargs(test_type, run_info_data, **kwargs):
     return {"binary": kwargs["binary"],
             "prefs_root": kwargs["prefs_root"],
             "extra_prefs": kwargs["extra_prefs"],
             "debug_info": kwargs["debug_info"],
             "symbols_path": kwargs["symbols_path"],
             "stackwalk_binary": kwargs["stackwalk_binary"],
             "certutil_binary": kwargs["certutil_binary"],
             "ca_certificate_path": kwargs["ssl_env"].ca_cert_path(),
             "e10s": kwargs["gecko_e10s"],
             "stackfix_dir": kwargs["stackfix_dir"],
-            "binary_args": kwargs["binary_args"]}
+            "binary_args": kwargs["binary_args"],
+            "timeout_multiplier": get_timeout_multiplier(test_type,
+                                                         run_info_data,
+                                                         **kwargs)}
 
 
 def executor_kwargs(test_type, server_config, cache_manager, run_info_data,
                     **kwargs):
     executor_kwargs = base_executor_kwargs(test_type, server_config,
                                            cache_manager, **kwargs)
     executor_kwargs["close_after_done"] = test_type != "reftest"
-    if kwargs["timeout_multiplier"] is None:
-        if test_type == "reftest":
-            if run_info_data["debug"] or run_info_data.get("asan"):
-                executor_kwargs["timeout_multiplier"] = 4
-            else:
-                executor_kwargs["timeout_multiplier"] = 2
-        elif run_info_data["debug"] or run_info_data.get("asan"):
-            executor_kwargs["timeout_multiplier"] = 3
+    executor_kwargs["timeout_multiplier"] = get_timeout_multiplier(test_type,
+                                                                   run_info_data,
+                                                                   **kwargs)
     if test_type == "wdspec":
         executor_kwargs["webdriver_binary"] = kwargs.get("webdriver_binary")
         fxOptions = {}
         if kwargs["binary"]:
             fxOptions["binary"] = kwargs["binary"]
         if kwargs["binary_args"]:
             fxOptions["args"] = kwargs["binary_args"]
         fxOptions["prefs"] = {
@@ -111,17 +122,17 @@ def update_properties():
 class FirefoxBrowser(Browser):
     used_ports = set()
     init_timeout = 60
     shutdown_timeout = 60
 
     def __init__(self, logger, binary, prefs_root, extra_prefs=None, debug_info=None,
                  symbols_path=None, stackwalk_binary=None, certutil_binary=None,
                  ca_certificate_path=None, e10s=False, stackfix_dir=None,
-                 binary_args=None):
+                 binary_args=None, timeout_multiplier=None):
         Browser.__init__(self, logger)
         self.binary = binary
         self.prefs_root = prefs_root
         self.extra_prefs = extra_prefs
         self.marionette_port = None
         self.runner = None
         self.debug_info = debug_info
         self.profile = None
@@ -131,16 +142,18 @@ class FirefoxBrowser(Browser):
         self.certutil_binary = certutil_binary
         self.e10s = e10s
         self.binary_args = binary_args
         if self.symbols_path and stackfix_dir:
             self.stack_fixer = get_stack_fixer_function(stackfix_dir,
                                                         self.symbols_path)
         else:
             self.stack_fixer = None
+        if timeout_multiplier:
+            self.init_timeout = self.init_timeout * timeout_multiplier
 
     def start(self):
         self.marionette_port = get_free_port(2828, exclude=self.used_ports)
         self.used_ports.add(self.marionette_port)
 
         env = os.environ.copy()
         env["MOZ_DISABLE_NONLOCAL_CONNECTIONS"] = "1"
 
--- a/testing/web-platform/harness/wptrunner/browsers/servo.py
+++ b/testing/web-platform/harness/wptrunner/browsers/servo.py
@@ -25,17 +25,17 @@ here = os.path.join(os.path.split(__file
     "update_properties": "update_properties",
 }
 
 
 def check_args(**kwargs):
     require_arg(kwargs, "binary")
 
 
-def browser_kwargs(**kwargs):
+def browser_kwargs(test_type, run_info_data, **kwargs):
     return {
         "binary": kwargs["binary"],
         "debug_info": kwargs["debug_info"],
         "binary_args": kwargs["binary_args"],
         "user_stylesheets": kwargs.get("user_stylesheets"),
     }
 
 
--- a/testing/web-platform/harness/wptrunner/browsers/servodriver.py
+++ b/testing/web-platform/harness/wptrunner/browsers/servodriver.py
@@ -37,17 +37,17 @@ 127.0.0.1 xn--n8j6ds53lwwkrqhv28a.web-pl
 127.0.0.1 xn--lve-6lad.web-platform.test
 """
 
 
 def check_args(**kwargs):
     require_arg(kwargs, "binary")
 
 
-def browser_kwargs(**kwargs):
+def browser_kwargs(test_type, run_info_data, **kwargs):
     return {
         "binary": kwargs["binary"],
         "debug_info": kwargs["debug_info"],
         "user_stylesheets": kwargs.get("user_stylesheets"),
     }
 
 
 def executor_kwargs(test_type, server_config, cache_manager, run_info_data, **kwargs):
--- a/testing/web-platform/harness/wptrunner/executors/executormarionette.py
+++ b/testing/web-platform/harness/wptrunner/executors/executormarionette.py
@@ -47,38 +47,41 @@ def do_delayed_imports():
     try:
         import marionette
         from marionette import errors
     except ImportError:
         from marionette_driver import marionette, errors
 
 
 class MarionetteProtocol(Protocol):
-    def __init__(self, executor, browser):
+    def __init__(self, executor, browser, timeout_multiplier=1):
         do_delayed_imports()
 
         Protocol.__init__(self, executor, browser)
         self.marionette = None
         self.marionette_port = browser.marionette_port
+        self.timeout_multiplier = timeout_multiplier
         self.timeout = None
         self.runner_handle = None
 
     def setup(self, runner):
         """Connect to browser via Marionette."""
         Protocol.setup(self, runner)
 
         self.logger.debug("Connecting to Marionette on port %i" % self.marionette_port)
+        startup_timeout = marionette.Marionette.DEFAULT_STARTUP_TIMEOUT * self.timeout_multiplier
         self.marionette = marionette.Marionette(host='localhost',
                                                 port=self.marionette_port,
-                                                socket_timeout=None)
+                                                socket_timeout=None,
+                                                startup_timeout=startup_timeout)
 
         # XXX Move this timeout somewhere
         self.logger.debug("Waiting for Marionette connection")
         while True:
-            success = self.marionette.wait_for_port(60)
+            success = self.marionette.wait_for_port(60 * self.timeout_multiplier)
             #When running in a debugger wait indefinitely for firefox to start
             if success or self.executor.debug_info is None:
                 break
 
         session_started = False
         if success:
             try:
                 self.logger.debug("Starting Marionette session")
@@ -403,17 +406,17 @@ class ExecuteAsyncScriptRun(object):
 class MarionetteTestharnessExecutor(TestharnessExecutor):
     def __init__(self, browser, server_config, timeout_multiplier=1,
                  close_after_done=True, debug_info=None, **kwargs):
         """Marionette-based executor for testharness.js tests"""
         TestharnessExecutor.__init__(self, browser, server_config,
                                      timeout_multiplier=timeout_multiplier,
                                      debug_info=debug_info)
 
-        self.protocol = MarionetteProtocol(self, browser)
+        self.protocol = MarionetteProtocol(self, browser, timeout_multiplier)
         self.script = open(os.path.join(here, "testharness_marionette.js")).read()
         self.close_after_done = close_after_done
         self.window_id = str(uuid.uuid4())
 
         self.original_pref_values = {}
 
         if marionette is None:
             do_delayed_imports()
--- a/testing/web-platform/harness/wptrunner/wptrunner.py
+++ b/testing/web-platform/harness/wptrunner/wptrunner.py
@@ -155,18 +155,16 @@ def run_tests(config, test_paths, produc
                                  kwargs["debug_info"],
                                  env_options) as test_environment:
             try:
                 test_environment.ensure_started()
             except env.TestEnvironmentError as e:
                 logger.critical("Error starting test environment: %s" % e.message)
                 raise
 
-            browser_kwargs = get_browser_kwargs(ssl_env=ssl_env, **kwargs)
-
             repeat = kwargs["repeat"]
             repeat_count = 0
             repeat_until_unexpected = kwargs["repeat_until_unexpected"]
 
             while repeat_count < repeat or repeat_until_unexpected:
                 repeat_count += 1
                 if repeat_until_unexpected:
                     logger.info("Repetition %i" % (repeat_count))
@@ -183,32 +181,37 @@ def run_tests(config, test_paths, produc
                     # processes are managed by a WebDriver server binary. This
                     # obviates the need for wptrunner to provide a browser, so
                     # the NullBrowser is used in place of the "target" browser
                     if test_type == "wdspec":
                         browser_cls = NullBrowser
                     else:
                         browser_cls = target_browser_cls
 
-                    for test in test_loader.disabled_tests[test_type]:
-                        logger.test_start(test.id)
-                        logger.test_end(test.id, status="SKIP")
+                    browser_kwargs = get_browser_kwargs(test_type,
+                                                        run_info,
+                                                        ssl_env=ssl_env,
+                                                        **kwargs)
+
 
                     executor_cls = executor_classes.get(test_type)
                     executor_kwargs = get_executor_kwargs(test_type,
                                                           test_environment.external_config,
                                                           test_environment.cache_manager,
                                                           run_info,
                                                           **kwargs)
 
                     if executor_cls is None:
                         logger.error("Unsupported test type %s for product %s" %
                                      (test_type, product))
                         continue
 
+                    for test in test_loader.disabled_tests[test_type]:
+                        logger.test_start(test.id)
+                        logger.test_end(test.id, status="SKIP")
 
                     with ManagerGroup("web-platform-tests",
                                       kwargs["processes"],
                                       test_source_cls,
                                       test_source_kwargs,
                                       browser_cls,
                                       browser_kwargs,
                                       executor_cls,