Bug 1414882 - Remove wait_for_port in favor of raise_for_port. draft
authorHenrik Skupin <mail@hskupin.info>
Wed, 08 Nov 2017 20:25:47 +0100
changeset 695129 39a494b4b617174daeeccb3a80ff5c02f5408765
parent 695128 e4a38d931a56536d2107795848f6af830bb551bc
child 695130 ffe86e2912fda955a3ce2cbd0034dbdeaf415047
child 695533 638380acfa0fa81b3d6e05406bdd7667d3ce9d21
child 695537 09bf86b4e48dc2606a95c82890624fa0c0a0e7cc
push id88360
push userbmo:hskupin@gmail.com
push dateWed, 08 Nov 2017 21:29:48 +0000
Bug 1414882 - Remove wait_for_port in favor of raise_for_port. wait_for_port() is defined to return a boolean value, which makes it impossible to return more detailed information in case of a connection cannot be established. Further the raise_for_port() method is a simple wrapper, without any additional usage. MozReview-Commit-ID: 2A4sCaEylgP
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -665,71 +665,65 @@ class Marionette(object):
             s.bind((host, port))
             return True
         except socket.error:
             return False
-    def wait_for_port(self, timeout=None):
-        """Wait until Marionette server has been created the communication socket.
+    def raise_for_port(self, timeout=None):
+        """Raise socket.timeout if no connection can be established.
         :param timeout: Timeout in seconds for the server to be ready.
         if timeout is None:
             timeout = self.DEFAULT_STARTUP_TIMEOUT
         runner = None
         if self.instance is not None:
             runner = self.instance.runner
         poll_interval = 0.1
         starttime = datetime.datetime.now()
+        timeout_time = starttime + datetime.timedelta(seconds=timeout)
-        timeout_time = starttime + datetime.timedelta(seconds=timeout)
+        connected = False
         while datetime.datetime.now() < timeout_time:
             # If the instance we want to connect to is not running return immediately
             if runner is not None and not runner.is_running():
-                return False
+                break
             sock = None
                 sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
                 sock.connect((self.host, self.port))
                 data = sock.recv(16)
                 # If the application starts up very slowly (eg. Fennec on Android
                 # emulator) a response package has to be received first. Otherwise
                 # start_session will fail (see bug 1410366 comment 32 ff.)
                 if ":" in data:
-                    return True
+                    connected = True
+                    break
             except socket.error:
                 if sock is not None:
-        return False
-    def raise_for_port(self, timeout=None):
-        """Raise socket.timeout if no connection can be established.
-        :param timeout: Timeout in seconds for the server to be ready.
-        """
-        if not self.wait_for_port(timeout):
+        if not connected:
             raise socket.timeout("Timed out waiting for connection on {0}:{1}!".format(
                 self.host, self.port))
     def _send_message(self, name, params=None, key=None):
         """Send a blocking message to the server.
         Marionette provides an asynchronous, non-blocking interface and
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_marionette.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_marionette.py
@@ -1,12 +1,13 @@
 # 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 socket
 import time
 from marionette_driver import errors
 from marionette_driver.marionette import Marionette
 from marionette_harness import MarionetteTestCase, run_if_manage_instance, skip_if_mobile
 class TestMarionette(MarionetteTestCase):
@@ -18,40 +19,39 @@ class TestMarionette(MarionetteTestCase)
         self.assertEqual(self.marionette.test_name, expected_test_name)
     @run_if_manage_instance("Only runnable if Marionette manages the instance")
     @skip_if_mobile("Bug 1322993 - Missing temporary folder")
-    def test_wait_for_port_non_existing_process(self):
-        """Test that wait_for_port doesn't run into a timeout if instance is not running."""
+    def test_raise_for_port_non_existing_process(self):
+        """Test that raise_for_port doesn't run into a timeout if instance is not running."""
         start_time = time.time()
-        self.assertFalse(self.marionette.wait_for_port(timeout=5))
+        self.assertRaises(socket.timeout, self.marionette.raise_for_port, timeout=5)
         self.assertLess(time.time() - start_time, 5)
     def test_disable_enable_new_connections(self):
         # Do not re-create socket if it already exists
         self.marionette._send_message("acceptConnections", {"value": True})
             # Disabling new connections does not affect existing ones...
             self.marionette._send_message("acceptConnections", {"value": False})
             self.assertEqual(1, self.marionette.execute_script("return 1"))
             # but only new connection attempts
             marionette = Marionette(host=self.marionette.host, port=self.marionette.port)
-            self.assertFalse(marionette.wait_for_port(timeout=1.0),
-                             "Unexpected connection with acceptConnections=false")
+            self.assertRaises(socket.timeout, marionette.raise_for_port, timeout=1.0)
             self.marionette._send_message("acceptConnections", {"value": True})
-            marionette.wait_for_port(timeout=1.0)
+            marionette.raise_for_port(timeout=1.0)
             self.marionette._send_message("acceptConnections", {"value": True})
 class TestContext(MarionetteTestCase):
     def setUp(self):