Bug 1414882 - Remove wait_for_port in favor of raise_for_port. r=ato
authorHenrik Skupin <mail@hskupin.info>
Wed, 08 Nov 2017 20:25:47 +0100
changeset 444542 48e08f18761ecc4492b21d24470ea93d5a0cef84
parent 444541 3c135c119ef48c70730959624519487974199987
child 444543 96cbaf1268ad66536431159e7a76a3916fcc7373
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1414882
milestone58.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 1414882 - Remove wait_for_port in favor of raise_for_port. r=ato 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
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/harness/marionette_harness/tests/unit/test_marionette.py
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -665,71 +665,65 @@ class Marionette(object):
         try:
             s.bind((host, port))
             return True
         except socket.error:
             return False
         finally:
             s.close()
 
-    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
             try:
                 sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
                 sock.settimeout(0.5)
                 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:
                 pass
             finally:
                 if sock is not None:
                     try:
                         sock.shutdown(socket.SHUT_RDWR)
                     except:
                         pass
                     sock.close()
 
             time.sleep(poll_interval)
 
-        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))
 
     @do_process_check
     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)
             cls=self.__class__.__name__,
             func=self.test_correct_test_name.__name__,
         )
 
         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."""
         self.marionette.quit()
         self.assertIsNotNone(self.marionette.instance.runner.returncode)
         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})
 
         try:
             # 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)
 
         finally:
             self.marionette._send_message("acceptConnections", {"value": True})
 
 
 class TestContext(MarionetteTestCase):
 
     def setUp(self):