Bug 1500242 - [marionette] Don't modify DEFAULT_SHUTDOWN_TIMEOUT constant. r=ato
authorHenrik Skupin <mail@hskupin.info>
Fri, 19 Oct 2018 11:54:13 +0000
changeset 442141 9645a734546be91c08cdba441d7003259e51bda8
parent 442140 f6c89db3520cca62fc39ec0915beecf5e9a22278
child 442142 9a2be0efbc4c4f1931f39a1dfe4a082cec09b2e0
push id34890
push userdvarga@mozilla.com
push dateSat, 20 Oct 2018 09:40:11 +0000
treeherdermozilla-central@d0f1450799b5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1500242
milestone64.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 1500242 - [marionette] Don't modify DEFAULT_SHUTDOWN_TIMEOUT constant. r=ato Differential Revision: https://phabricator.services.mozilla.com/D9227
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -562,17 +562,17 @@ class Alert(object):
 
 
 class Marionette(object):
     """Represents a Marionette connection to a browser or device."""
 
     CONTEXT_CHROME = "chrome"  # non-browser content: windows, dialogs, etc.
     CONTEXT_CONTENT = "content"  # browser content: iframes, divs, etc.
     DEFAULT_STARTUP_TIMEOUT = 120
-    DEFAULT_SHUTDOWN_TIMEOUT = 120  # Firefox will kill hanging threads after 60s
+    DEFAULT_SHUTDOWN_TIMEOUT = 70  # By default Firefox will kill hanging threads after 60s
 
     # 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="127.0.0.1", port=2828, app=None, bin=None,
@@ -621,16 +621,18 @@ class Marionette(object):
         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)
 
+        self.shutdown_timeout = self.DEFAULT_SHUTDOWN_TIMEOUT
+
         if self.bin:
             self.instance = GeckoInstance.create(
                 app, host=self.host, port=self.port, bin=self.bin, **instance_args)
             self.start_binary(self.startup_timeout)
 
         self.timeout = Timeouts(self)
 
     @property
@@ -811,17 +813,17 @@ class Marionette(object):
         # If the application hasn't been launched by Marionette no further action can be done.
         # In such cases we simply re-throw the exception.
         if not self.instance:
             reraise(exc, val, tb)
 
         else:
             # Somehow the socket disconnected. Give the application some time to shutdown
             # itself before killing the process.
-            returncode = self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
+            returncode = self.instance.runner.wait(timeout=self.shutdown_timeout)
 
             if returncode is None:
                 message = ('Process killed because the connection to Marionette server is '
                            'lost. Check gecko.log for errors')
                 # This will force-close the application without sending any other message.
                 self.cleanup()
             else:
                 # If Firefox quit itself check if there was a crash
@@ -1112,23 +1114,23 @@ class Marionette(object):
                     cause = self._request_in_app_shutdown()
 
             except IOError:
                 # A possible IOError should be ignored at this point, given that
                 # quit() could have been called inside of `using_context`,
                 # which wants to reset the context but fails sending the message.
                 pass
 
-            returncode = self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
+            returncode = self.instance.runner.wait(timeout=self.shutdown_timeout)
             if returncode is None:
                 # The process did not shutdown itself, so force-closing it.
                 self.cleanup()
 
                 message = "Process still running {}s after quit request"
-                raise IOError(message.format(self.DEFAULT_SHUTDOWN_TIMEOUT))
+                raise IOError(message.format(self.shutdown_timeout))
 
             self.is_shutting_down = False
             self.delete_session(send_request=False)
 
         else:
             self.delete_session(send_request=False)
             self.instance.close(clean=clean)
 
@@ -1180,29 +1182,29 @@ class Marionette(object):
                 # A possible IOError should be ignored at this point, given that
                 # restart() could have been called inside of `using_context`,
                 # which wants to reset the context but fails sending the message.
                 pass
 
             try:
                 # Wait for a new Marionette connection to appear while the
                 # process restarts itself.
-                self.raise_for_port(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT,
+                self.raise_for_port(timeout=self.shutdown_timeout,
                                     check_process_status=False)
             except socket.timeout:
                 exc, val, tb = sys.exc_info()
 
                 if self.instance.runner.returncode is None:
                     # The process is still running, which means the shutdown
                     # request was not correct or the application ignored it.
                     # Allow Marionette to accept connections again.
                     self._send_message("acceptConnections", {"value": True})
 
                     message = "Process still running {}s after restart request"
-                    reraise(exc, message.format(self.DEFAULT_SHUTDOWN_TIMEOUT), tb)
+                    reraise(exc, message.format(self.shutdown_timeout), tb)
 
                 else:
                     # The process shutdown but didn't start again.
                     self.cleanup()
                     msg = "Process unexpectedly quit without restarting (exit code: {})"
                     reraise(exc, msg.format(self.instance.runner.returncode), tb)
 
             finally:
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
@@ -195,36 +195,36 @@ class TestQuitRestart(MarionetteTestCase
                             "about:about")
 
     def test_in_app_restart_with_callback_not_callable(self):
         with self.assertRaisesRegexp(ValueError, "is not callable"):
             self.marionette.restart(in_app=True, callback=4)
 
     @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_in_app_restart_with_callback_but_process_quit(self):
-        timeout_shutdown = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
+        timeout_shutdown = self.marionette.shutdown_timeout
         # Wait at least 70s for the hang monitor in case of a shutdown hang
-        self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 70
+        self.marionette.shutdown_timeout = 70
 
         try:
             with self.assertRaisesRegexp(IOError, "Process unexpectedly quit without restarting"):
                 self.marionette.restart(in_app=True, callback=lambda: self.shutdown(restart=False))
         finally:
-            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = timeout_shutdown
+            self.marionette.shutdown_timeout = timeout_shutdown
 
     @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_in_app_restart_with_callback_missing_shutdown(self):
         try:
-            timeout_shutdown = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
-            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 5
+            timeout_shutdown = self.marionette.shutdown_timeout
+            self.marionette.shutdown_timeout = 5
 
             with self.assertRaisesRegexp(IOError, "the connection to Marionette server is lost"):
                 self.marionette.restart(in_app=True, callback=lambda: False)
         finally:
-            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = timeout_shutdown
+            self.marionette.shutdown_timeout = timeout_shutdown
 
     @skip("Bug 1397612 - Hang of Marionette client after the restart")
     def test_in_app_restart_safe_mode(self):
 
         def restart_in_safe_mode():
             with self.marionette.using_context("chrome"):
                 self.marionette.execute_script("""
                   Components.utils.import("resource://gre/modules/Services.jsm");
@@ -270,23 +270,23 @@ class TestQuitRestart(MarionetteTestCase
         self.marionette.start_session()
         self.assertEqual(self.marionette.profile, self.profile)
         self.assertNotEqual(self.marionette.session_id, self.session_id)
         self.assertNotEqual(self.marionette.get_pref("startup.homepage_welcome_url"),
                             "about:about")
 
     def test_in_app_quit_with_callback_missing_shutdown(self):
         try:
-            timeout = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
-            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 5
+            timeout = self.marionette.shutdown_timeout
+            self.marionette.shutdown_timeout = 5
 
             with self.assertRaisesRegexp(IOError, "Process still running"):
                 self.marionette.quit(in_app=True, callback=lambda: False)
         finally:
-            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = timeout
+            self.marionette.shutdown_timeout = timeout
 
     def test_in_app_quit_with_callback_not_callable(self):
         with self.assertRaisesRegexp(ValueError, "is not callable"):
             self.marionette.restart(in_app=True, callback=4)
 
     def test_in_app_quit_with_dismissed_beforeunload_prompt(self):
         self.marionette.navigate(inline("""
           <input type="text">