Bug 1397734 - Centralize defaults for socket and startup timeouts r=maja_zf
authorHenrik Skupin <mail@hskupin.info>
Thu, 07 Sep 2017 15:36:50 +0200
changeset 429206 629ecdd561c4ac05052de7a0703dc282fbb411c6
parent 429205 0181bf16af4f45ddab48bf2e7d3b0410a17c0d08
child 429207 9815926c3bc14b22941415a5e036d1be2bc87fdf
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmaja_zf
bugs1397734
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1397734 - Centralize defaults for socket and startup timeouts r=maja_zf Currently defaults for startup_timeout and socket_timeout are defined at two different places (Marionette driver and harness). As of now it's even the case that startup_timeout has different values. While Marionette driver uses 120s, the harness only uses 60s. As result all jobs which are based on the Marionette harness fail if Firefox starts-up slowly like for debug builds. MozReview-Commit-ID: Dl4sBG1H7NA
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/harness/marionette_harness/runner/base.py
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -558,17 +558,17 @@ class Marionette(object):
 
     # Bug 1336953 - Until we can remove the socket timeout parameter it has to be
     # set a default value which is larger than the longest timeout as defined by the
     # WebDriver spec. In that case its 300s for page load. Also add another minute
     # so that slow builds have enough time to send the timeout error to the client.
     DEFAULT_SOCKET_TIMEOUT = 360
 
     def __init__(self, host="localhost", port=2828, app=None, bin=None,
-                 baseurl=None, socket_timeout=DEFAULT_SOCKET_TIMEOUT,
+                 baseurl=None, socket_timeout=None,
                  startup_timeout=None, **instance_args):
         """Construct a holder for the Marionette connection.
 
         Remember to call ``start_session`` in order to initiate the
         connection and start a Marionette session.
 
         :param host: Host where the Marionette server listens.
             Defaults to localhost.
@@ -595,20 +595,28 @@ class Marionette(object):
         self.session = None
         self.session_id = None
         self.process_id = None
         self.profile = None
         self.window = None
         self.chrome_window = None
         self.baseurl = baseurl
         self._test_name = None
-        self.socket_timeout = socket_timeout
         self.crashed = 0
 
-        self.startup_timeout = int(startup_timeout or self.DEFAULT_STARTUP_TIMEOUT)
+        if socket_timeout is None:
+            self.socket_timeout = self.DEFAULT_SOCKET_TIMEOUT
+        else:
+            self.socket_timeout = float(socket_timeout)
+
+        if startup_timeout is None:
+            self.startup_timeout = self.DEFAULT_STARTUP_TIMEOUT
+        else:
+            self.startup_timeout = int(startup_timeout)
+
         if self.bin:
             if not Marionette.is_port_available(self.port, host=self.host):
                 ex_msg = "{0}:{1} is unavailable.".format(self.host, self.port)
                 raise errors.MarionetteException(message=ex_msg)
 
             self.instance = GeckoInstance.create(
                 app, host=self.host, port=self.port, bin=self.bin, **instance_args)
             self.instance.start()
--- a/testing/marionette/harness/marionette_harness/runner/base.py
+++ b/testing/marionette/harness/marionette_harness/runner/base.py
@@ -234,21 +234,16 @@ class MarionetteTextTestRunner(Structure
 
     def run(self, test):
         result = super(MarionetteTextTestRunner, self).run(test)
         result.printLogs(test)
         return result
 
 
 class BaseMarionetteArguments(ArgumentParser):
-    # Bug 1336953 - Until we can remove the socket timeout parameter it has to be
-    # set a default value which is larger than the longest timeout as defined by the
-    # WebDriver spec. In that case its 300s for page load. Also add another minute
-    # so that slow builds have enough time to send the timeout error to the client.
-    socket_timeout_default = 360.0
 
     def __init__(self, **kwargs):
         ArgumentParser.__init__(self, **kwargs)
 
         def dir_path(path):
             path = os.path.abspath(os.path.expanduser(path))
             if not os.access(path, os.F_OK):
                 os.makedirs(path)
@@ -307,21 +302,26 @@ class BaseMarionetteArguments(ArgumentPa
                                "Default cap is 30 runs, which can be overwritten "
                                "with the --repeat parameter.")
         self.add_argument('--testvars',
                           action='append',
                           help='path to a json file with any test data required')
         self.add_argument('--symbols-path',
                           help='absolute path to directory containing breakpad symbols, or the '
                                'url of a zip file containing symbols')
+        self.add_argument('--socket-timeout',
+                          type=float,
+                          default=Marionette.DEFAULT_SOCKET_TIMEOUT,
+                          help='Set the global timeout for marionette socket operations.'
+                               ' Default: %(default)ss.')
         self.add_argument('--startup-timeout',
                           type=int,
-                          default=60,
+                          default=Marionette.DEFAULT_STARTUP_TIMEOUT,
                           help='the max number of seconds to wait for a Marionette connection '
-                               'after launching a binary')
+                               'after launching a binary. Default: %(default)ss.')
         self.add_argument('--shuffle',
                           action='store_true',
                           default=False,
                           help='run tests in a random order')
         self.add_argument('--shuffle-seed',
                           type=int,
                           default=random.randint(0, sys.maxint),
                           help='Use given seed to shuffle tests')
@@ -346,21 +346,16 @@ class BaseMarionetteArguments(ArgumentPa
                           help='Define the name to associate with the logger used')
         self.add_argument('--jsdebugger',
                           action='store_true',
                           default=False,
                           help='Enable the jsdebugger for marionette javascript.')
         self.add_argument('--pydebugger',
                           help='Enable python post-mortem debugger when a test fails.'
                                ' Pass in the debugger you want to use, eg pdb or ipdb.')
-        self.add_argument('--socket-timeout',
-                          type=float,
-                          default=self.socket_timeout_default,
-                          help='Set the global timeout for marionette socket operations.'
-                               ' Default: %(default)ss.')
         self.add_argument('--disable-e10s',
                           action='store_false',
                           dest='e10s',
                           default=True,
                           help='Disable e10s when running marionette tests.')
         self.add_argument("--headless",
                           action="store_true",
                           dest="headless",
@@ -512,18 +507,19 @@ class BaseMarionetteTestRunner(object):
                  repeat=None,
                  run_until_failure=None,
                  testvars=None,
                  symbols_path=None,
                  shuffle=False, shuffle_seed=random.randint(0, sys.maxint), this_chunk=1,
                  total_chunks=1,
                  server_root=None, gecko_log=None, result_callbacks=None,
                  prefs=None, test_tags=None,
-                 socket_timeout=BaseMarionetteArguments.socket_timeout_default,
-                 startup_timeout=None, addons=None, workspace=None,
+                 socket_timeout=None,
+                 startup_timeout=None,
+                 addons=None, workspace=None,
                  verbose=0, e10s=True, emulator=False, headless=False, **kwargs):
         self._appName = None
         self._capabilities = None
         self._filename_pattern = None
         self._version_info = {}
 
         self.fixture_servers = {}
         self.fixtures = Fixtures()
@@ -538,28 +534,28 @@ class BaseMarionetteTestRunner(object):
         self.addons = addons
         self.logger = logger
         self.marionette = None
         self.logdir = logdir
         self.repeat = repeat or 0
         self.run_until_failure = run_until_failure or False
         self.symbols_path = symbols_path
         self.socket_timeout = socket_timeout
+        self.startup_timeout = startup_timeout
         self.shuffle = shuffle
         self.shuffle_seed = shuffle_seed
         self.server_root = server_root
         self.this_chunk = this_chunk
         self.total_chunks = total_chunks
         self.mixin_run_tests = []
         self.manifest_skipped_tests = []
         self.tests = []
         self.result_callbacks = result_callbacks or []
         self.prefs = prefs or {}
         self.test_tags = test_tags
-        self.startup_timeout = startup_timeout
         self.workspace = workspace
         # If no workspace is set, default location for gecko.log is .
         # and default location for profile is TMP
         self.workspace_path = workspace or os.getcwd()
         self.verbose = verbose
         self.headless = headless
 
         # self.e10s stores the desired configuration, whereas