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 695735 c661cb83ab9e908ba50ce501c5e80e073810b9a2
parent 694943 f63559d7e6a570e4e73ba367964099394248e18d
child 695736 6a7e499f5ea8446ee5a4b4281c3bfd6493ae0130
child 695740 dda43afaf669f182e33e1612528c2830833602a3
child 696466 15196f095be99c8b520f71138972f9cb8e860d86
child 696475 dd9d8ce8c4dc247e1f0db969fccd96112169efe4
push id88526
push userbmo:hskupin@gmail.com
push dateThu, 09 Nov 2017 19:44:27 +0000
bugs1414882
milestone58.0a1
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
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):