Bug 1397734 - Centralize defaults for socket and startup timeouts draft
authorHenrik Skupin <mail@hskupin.info>
Thu, 07 Sep 2017 15:36:50 +0200
changeset 661427 fe6f0f1dffc0f3ba346a4b864b0cfc12877f7c69
parent 661321 50857982881ae7803ceb438fee90650a282f7f05
child 730564 ecab7382dda6bc9a64c2e64bbcddff094d9a8598
push id78751
push userbmo:hskupin@gmail.com
push dateFri, 08 Sep 2017 12:11:36 +0000
bugs1397734
milestone57.0a1
Bug 1397734 - Centralize defaults for socket and startup timeouts 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