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 490488 9645a734546be91c08cdba441d7003259e51bda8
parent 490487 f6c89db3520cca62fc39ec0915beecf5e9a22278
child 490489 9a2be0efbc4c4f1931f39a1dfe4a082cec09b2e0
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersato
bugs1500242
milestone64.0a1
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">