Bug 1290372 - wait_for_port() has to return early if instance is not running.
In case when the instance is not running, the method would check for the port again
and again until the timeout is reached. This extra time can be spent if the process
status is checked.
MozReview-Commit-ID: C2WAWNC5CWE
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -1,18 +1,20 @@
# 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 ConfigParser
import base64
-import ConfigParser
+import datetime
import json
import os
import socket
import sys
+import time
import traceback
import warnings
from contextlib import contextmanager
from decorators import do_process_check
from keys import Keys
@@ -570,17 +572,17 @@ class Marionette(object):
self._test_name = None
self.timeout = timeout
self.socket_timeout = socket_timeout
startup_timeout = startup_timeout or self.DEFAULT_STARTUP_TIMEOUT
if self.bin:
self.instance = self._create_instance(app, instance_args)
self.instance.start()
- self.raise_for_port(self.wait_for_port(timeout=startup_timeout))
+ self.raise_for_port(timeout=startup_timeout)
def _create_instance(self, app, instance_args):
if not Marionette.is_port_available(self.port, host=self.host):
ex_msg = "{0}:{1} is unavailable.".format(self.host, self.port)
raise errors.MarionetteException(message=ex_msg)
if app:
# select instance class for the given app
try:
@@ -635,23 +637,64 @@ class Marionette(object):
s.bind((host, port))
return True
except socket.error:
return False
finally:
s.close()
def wait_for_port(self, timeout=None):
- timeout = timeout or self.DEFAULT_STARTUP_TIMEOUT
- return transport.wait_for_port(self.host, self.port, timeout=timeout)
+ """Wait until Marionette server has been created the communication socket.
+
+ :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()
+
+ while datetime.datetime.now() - starttime < datetime.timedelta(seconds=timeout):
+ # 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
+
+ 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 ":" in data:
+ return True
+ except socket.error:
+ pass
+ finally:
+ if sock is not None:
+ sock.close()
+
+ time.sleep(poll_interval)
+
+ return False
@do_process_check
- def raise_for_port(self, port_obtained):
- if not port_obtained:
- raise socket.timeout("Timed out waiting for port {}!".format(self.port))
+ 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):
+ 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
this attempts to paper over this by providing a synchronous API
to the user.
@@ -1027,17 +1070,17 @@ class Marionette(object):
}}
""".format(pref, value))
if not pref_exists:
break
self.set_context(self.CONTEXT_CONTENT)
if not pref_exists:
self.delete_session()
self.instance.restart(prefs)
- self.raise_for_port(self.wait_for_port())
+ self.raise_for_port()
self.start_session()
self.reset_timeouts()
def _request_in_app_shutdown(self, shutdown_flags=None):
"""Terminate the currently running instance from inside the application.
:param shutdown_flags: If specified use additional flags for the shutdown
of the application. Possible values here correspond
@@ -1120,27 +1163,27 @@ class Marionette(object):
raise ValueError("An in_app restart cannot be triggered with the clean flag set")
if callable(callback):
callback()
else:
self._request_in_app_shutdown("eRestart")
try:
- self.raise_for_port(self.wait_for_port())
+ self.raise_for_port()
except socket.timeout:
if self.instance.runner.returncode is not None:
exc, val, tb = sys.exc_info()
self.cleanup()
raise exc, "Requested restart of the application was aborted", tb
else:
self.delete_session()
self.instance.restart(clean=clean)
- self.raise_for_port(self.wait_for_port())
+ self.raise_for_port()
self.start_session(session_id=session_id)
self.reset_timeouts()
if in_app and self.session.get("processId"):
# In some cases Firefox restarts itself by spawning into a new process group.
# As long as mozprocess cannot track that behavior (bug 1284864) we assist by
# informing about the new process id.
--- a/testing/marionette/client/marionette_driver/transport.py
+++ b/testing/marionette/client/marionette_driver/transport.py
@@ -1,13 +1,12 @@
# 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 datetime
import json
import socket
import time
class SocketTimeout(object):
def __init__(self, socket, timeout):
self.sock = socket
@@ -278,32 +277,8 @@ class TcpTransport(object):
"""Close the socket."""
if self.sock:
self.sock.shutdown(socket.SHUT_RDWR)
self.sock.close()
self.sock = None
def __del__(self):
self.close()
-
-
-def wait_for_port(host, port, timeout=60):
- """Wait for the specified host/port to become available."""
- starttime = datetime.datetime.now()
- poll_interval = 0.1
- while datetime.datetime.now() - starttime < datetime.timedelta(seconds=timeout):
- sock = None
- try:
- sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
- sock.settimeout(0.5)
- sock.connect((host, port))
- data = sock.recv(16)
- sock.shutdown(socket.SHUT_RDWR)
- sock.close()
- if ":" in data:
- return True
- except socket.error:
- pass
- finally:
- if sock is not None:
- sock.close()
- time.sleep(poll_interval)
- return False
--- a/testing/marionette/harness/marionette/tests/unit/test_marionette.py
+++ b/testing/marionette/harness/marionette/tests/unit/test_marionette.py
@@ -1,30 +1,40 @@
# 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 itertools
+import time
+
+from marionette.marionette_test import MarionetteTestCase
from marionette_driver import errors
-from marionette.marionette_test import MarionetteTestCase
-class TestMarionetteProperties(MarionetteTestCase):
+class TestMarionette(MarionetteTestCase):
def test_correct_test_name(self):
"""Test that the correct test name gets set."""
expected_test_name = '{module}.py {cls}.{func}'.format(
module=__name__,
cls=self.__class__.__name__,
func=self.test_correct_test_name.__name__,
)
self.assertEqual(self.marionette.test_name, expected_test_name)
+ 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."""
+ self.marionette.quit()
+ self.assertIsNotNone(self.marionette.instance.runner.returncode)
+ start_time = time.time()
+ self.assertFalse(self.marionette.wait_for_port(timeout=5))
+ self.assertLess(time.time() - start_time, 5)
+
class TestProtocol1Errors(MarionetteTestCase):
def setUp(self):
MarionetteTestCase.setUp(self)
self.op = self.marionette.protocol
self.marionette.protocol = 1
def tearDown(self):