Bug 1316622 - Add timeout reset API to Marionette Python client; r?whimboo,automatedtester draft
authorAndreas Tolfsen <ato@mozilla.com>
Thu, 17 Nov 2016 13:28:20 +0000
changeset 442879 b6d68e96f4ffa48ae061c3b329182b32a71494af
parent 442878 529b127db41dfa6fde3c3c728749863c8319a7f2
child 537922 381f1e138a9c7156bd7bbda9f2245a5d6f2f0390
push id36854
push userbmo:ato@mozilla.com
push dateWed, 23 Nov 2016 13:38:54 +0000
reviewerswhimboo, automatedtester
bugs1316622
milestone53.0a1
Bug 1316622 - Add timeout reset API to Marionette Python client; r?whimboo,automatedtester This removes the `default_timeouts` internal state associated with the Marionette client connection and introduces a new API to reset all timeouts to their documented defaults in `marionette_driver.timeouts.Timeout`. MozReview-Commit-ID: 3xFWsYLngMp
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/client/marionette_driver/timeout.py
testing/marionette/harness/marionette/marionette_test/testcases.py
testing/marionette/harness/marionette/runner/base.py
testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py
testing/marionette/harness/marionette/tests/unit/test_timeouts.py
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -539,47 +539,43 @@ class Marionette(object):
 
     CONTEXT_CHROME = "chrome"  # non-browser content: windows, dialogs, etc.
     CONTEXT_CONTENT = "content"  # browser content: iframes, divs, etc.
     DEFAULT_SOCKET_TIMEOUT = 60
     DEFAULT_STARTUP_TIMEOUT = 120
     DEFAULT_SHUTDOWN_TIMEOUT = 65  # Firefox will kill hanging threads after 60s
 
     def __init__(self, host="localhost", port=2828, app=None, bin=None,
-                 baseurl=None, timeout=None, socket_timeout=DEFAULT_SOCKET_TIMEOUT,
+                 baseurl=None, socket_timeout=DEFAULT_SOCKET_TIMEOUT,
                  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.
         :param port: Port where the Marionette server listens.
             Defaults to port 2828.
         :param baseurl: Where to look for files served from Marionette's
             www directory.
-        :param timeout: Dictionary of default page load, script, and
-            implicit wait timeouts.  Timeouts in the session are reset
-            to these values whenever ``reset_timeouts`` is called.
         :param socket_timeout: Timeout for Marionette socket operations.
         :param startup_timeout: Seconds to wait for a connection with
             binary.
         :param bin: Path to browser binary.  If any truthy value is given
             this will attempt to start a Gecko instance with the specified
             `app`.
         :param app: Type of ``instance_class`` to use for managing app
             instance. See ``marionette_driver.geckoinstance``.
         :param instance_args: Arguments to pass to ``instance_class``.
 
         """
         self.host = host
         self.port = self.local_port = int(port)
         self.bin = bin
-        self.default_timeouts = timeout
         self.instance = None
         self.session = None
         self.session_id = None
         self.window = None
         self.chrome_window = None
         self.baseurl = baseurl
         self._test_name = None
         self.socket_timeout = socket_timeout
@@ -759,32 +755,16 @@ class Marionette(object):
 
         else:
             error = obj["error"]
             message = obj["message"]
             stacktrace = obj["stacktrace"]
 
         raise errors.lookup(error)(message, stacktrace=stacktrace)
 
-    def reset_timeouts(self):
-        """Resets timeouts to their defaults to the
-        `self.default_timeouts` attribute. If unset, only the page load
-        timeout is reset to 30 seconds.
-
-        """
-        setters = {"search": "implicit",
-                   "script": "script",
-                   "page load": "page_load"}
-
-        if self.default_timeouts is not None:
-            for typ, ms in self.default_timeouts:
-                setattr(self.timeout, setters[typ], ms)
-        else:
-            self.timeout.page_load = 30
-
     def check_for_crash(self):
         """Check if the process crashed.
 
         :returns: True, if a crash happened since the method has been called the last time.
         """
         crash_count = 0
 
         if self.instance:
@@ -1123,17 +1103,17 @@ class Marionette(object):
                     break
 
         if not pref_exists:
             context = self._send_message("getContext", key="value")
             self.delete_session()
             self.instance.restart(prefs)
             self.raise_for_port()
             self.start_session()
-            self.reset_timeouts()
+            self.timeout.reset()
 
             # Restore the context as used before the restart
             self.set_context(context)
 
     def _request_in_app_shutdown(self, shutdown_flags=None):
         """Terminate the currently running instance from inside the application.
 
         :param shutdown_flags: If specified use additional flags for the shutdown
@@ -1172,17 +1152,17 @@ class Marionette(object):
                        by killing the process.
         :param callback: If provided and `in_app` is True, the callback will
                          be used to trigger the shutdown.
         """
         if not self.instance:
             raise errors.MarionetteException("quit() can only be called "
                                              "on Gecko instances launched by Marionette")
 
-        self.reset_timeouts()
+        self.timeout.reset()
 
         if in_app:
             if callable(callback):
                 self._send_message("acceptConnections", {"value": False})
                 callback()
             else:
                 self._request_in_app_shutdown()
 
@@ -1239,17 +1219,17 @@ class Marionette(object):
                     raise exc, "Requested restart of the application was aborted", tb
 
         else:
             self.delete_session()
             self.instance.restart(clean=clean)
             self.raise_for_port()
 
         self.start_session(session_id=session_id)
-        self.reset_timeouts()
+        self.timeout.reset()
 
         # Restore the context as used before the restart
         self.set_context(context)
 
         if in_app and self.session.get("processId"):
             # In some cases Firefox restarts itself by spawning into a new process group.
             # As long as mozprocess cannot track that behavior (bug 1284864) we assist by
             # informing about the new process id.
--- a/testing/marionette/client/marionette_driver/timeout.py
+++ b/testing/marionette/client/marionette_driver/timeout.py
@@ -1,16 +1,21 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 
 import errors
 
 
+DEFAULT_SCRIPT_TIMEOUT = 30
+DEFAULT_PAGE_LOAD_TIMEOUT = 300
+DEFAULT_IMPLICIT_WAIT_TIMEOUT = 0
+
+
 class Timeouts(object):
     """Manage timeout settings in the Marionette session.
 
     Usage::
 
         marionette = Marionette(...)
         marionette.start_session()
         marionette.timeout.page_load = 10
@@ -80,8 +85,14 @@ class Timeouts(object):
     @implicit.setter
     def implicit(self, sec):
         """Set the session's implicit wait timeout.  This specifies the
         time to wait for the implicit element location strategy when
         retrieving elements.
 
         """
         self._set("implicit", sec)
+
+    def reset(self):
+        """Resets timeouts to their default values."""
+        self.script = DEFAULT_SCRIPT_TIMEOUT
+        self.page_load = DEFAULT_PAGE_LOAD_TIMEOUT
+        self.implicit = DEFAULT_IMPLICIT_WAIT_TIMEOUT
--- a/testing/marionette/harness/marionette/marionette_test/testcases.py
+++ b/testing/marionette/harness/marionette/marionette_test/testcases.py
@@ -250,17 +250,17 @@ class CommonTestCase(unittest.TestCase):
         # duration of the test; this is deleted in tearDown() to prevent
         # a persistent circular reference which in turn would prevent
         # proper garbage collection.
         self.start_time = time.time()
         self.marionette = self._marionette_weakref()
         self.httpd = self._httpd_weakref()
         if self.marionette.session is None:
             self.marionette.start_session()
-        self.marionette.reset_timeouts()
+        self.marionette.timeout.reset()
 
         super(CommonTestCase, self).setUp()
 
     def cleanTest(self):
         self._deleteSession()
 
     def _deleteSession(self):
         if hasattr(self, 'start_time'):
--- a/testing/marionette/harness/marionette/runner/base.py
+++ b/testing/marionette/harness/marionette/runner/base.py
@@ -303,22 +303,16 @@ class BaseMarionetteArguments(ArgumentPa
                           default=0,
                           help='number of times to repeat the test(s)')
         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('--timeout',
-                          type=int,
-                          help='if a --timeout value is given, it will set the default page load '
-                               'timeout, search timeout and script timeout to the given value. '
-                               'If not passed in, it will use the default values of 30000ms for '
-                               'page load, 0ms for search timeout and 10000ms for script timeout')
         self.add_argument('--startup-timeout',
                           type=int,
                           default=60,
                           help='the max number of seconds to wait for a Marionette connection '
                                'after launching a binary')
         self.add_argument('--shuffle',
                           action='store_true',
                           default=False,
@@ -503,17 +497,17 @@ class BaseMarionetteTestRunner(object):
 
     textrunnerclass = MarionetteTextTestRunner
     driverclass = Marionette
 
     def __init__(self, address=None,
                  app=None, app_args=None, binary=None, profile=None,
                  logger=None, logdir=None,
                  repeat=0, testvars=None,
-                 symbols_path=None, timeout=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,
                  verbose=0, e10s=True, emulator=False, **kwargs):
 
@@ -533,17 +527,16 @@ class BaseMarionetteTestRunner(object):
         self.profile = profile
         self.addons = addons
         self.logger = logger
         self.httpd = None
         self.marionette = None
         self.logdir = logdir
         self.repeat = repeat
         self.symbols_path = symbols_path
-        self.timeout = timeout
         self.socket_timeout = socket_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 = []
@@ -718,17 +711,16 @@ class BaseMarionetteTestRunner(object):
         self.skipped = 0
         self.failures = []
 
     def _build_kwargs(self):
         if self.logdir and not os.access(self.logdir, os.F_OK):
             os.mkdir(self.logdir)
 
         kwargs = {
-            'timeout': self.timeout,
             'socket_timeout': self.socket_timeout,
             'prefs': self.prefs,
             'startup_timeout': self.startup_timeout,
             'verbose': self.verbose,
             'symbols_path': self.symbols_path,
         }
         if self.bin or self.emulator:
             kwargs.update({
--- a/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py
+++ b/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py
@@ -117,17 +117,17 @@ def test_args_passed_to_driverclass(mock
 
 
 def test_build_kwargs_basic_args(build_kwargs_using):
     '''Test the functionality of runner._build_kwargs:
     make sure that basic arguments (those which should
     always be included, irrespective of the runner's settings)
     get passed to the call to runner.driverclass'''
 
-    basic_args = ['timeout', 'socket_timeout', 'prefs',
+    basic_args = ['socket_timeout', 'prefs',
                   'startup_timeout', 'verbose', 'symbols_path']
     built_kwargs = build_kwargs_using([(a, getattr(sentinel, a)) for a in basic_args])
     for arg in basic_args:
         assert built_kwargs[arg] is getattr(sentinel, arg)
 
 
 @pytest.mark.parametrize('workspace', ['path/to/workspace', None])
 def test_build_kwargs_with_workspace(build_kwargs_using, workspace):
--- a/testing/marionette/harness/marionette/tests/unit/test_timeouts.py
+++ b/testing/marionette/harness/marionette/tests/unit/test_timeouts.py
@@ -8,17 +8,17 @@ from marionette_driver.errors import (No
                                       MarionetteException,
                                       InvalidArgumentException,
                                       ScriptTimeoutException)
 from marionette_driver.by import By
 
 
 class TestTimeouts(MarionetteTestCase):
     def tearDown(self):
-        self.marionette.reset_timeouts()
+        self.marionette.timeout.reset()
         MarionetteTestCase.tearDown(self)
 
     def test_page_timeout_notdefinetimeout_pass(self):
         test_html = self.marionette.absolute_url("test.html")
         self.marionette.navigate(test_html)
 
     def test_page_timeout_fail(self):
         self.marionette.timeout.page_load = 0