Merge pull request pytest-dev#126 from manahl/xfang-fix-get-port

pytest-server-fixtures: fix random port logic in TestServerV2

CHANGES.md

@@ -4,6 +4,7 @@
 ### 1.6.1 (Unrelease)
  * pytest-server-fixtures: fix exception when attempting to access hostname while server is not started
  * pytest-server-fixtures: suppress stacktrace if kill() is called
+ * pytest-server-fixtures: fix random port logic in TestServerV2
 
 ### 1.6.0 (2019-02-12)
  * pytest-server-fixtures: added previously removed TestServerV2.kill() function

pytest-server-fixtures/pytest_server_fixtures/base.py

@@ -89,6 +89,7 @@ def get_ephemeral_port(port=0, host=None, cache_host=True):
     while True:
         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         try:
+            s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
             s.bind((host, port))
             port = s.getsockname()[1]
             s.close()

pytest-server-fixtures/pytest_server_fixtures/base2.py

@@ -6,7 +6,7 @@
 
 from pytest_server_fixtures import CONFIG
 from pytest_shutil.workspace import Workspace
-from .base import get_ephemeral_port
+from .base import get_ephemeral_host, get_ephemeral_port
 from .serverclass import create_server
 
 log = logging.getLogger(__name__)
@@ -37,6 +37,7 @@ def __init__(self, cwd=None, workspace=None, delete=None, server_class=CONFIG.se
         self._server_class = server_class
         self._server = None
         self._killed = False
+        self._listen_hostname = self._get_hostname()
 
     def start(self):
         """
@@ -57,7 +58,7 @@ def start(self):
                 labels=self.labels,
                 workspace=self.workspace,
                 cwd=self._cwd,
-                random_hostname=self.random_hostname,
+                listen_hostname=self._listen_hostname,
             )
 
             if self._server_class == 'thread':
@@ -169,11 +170,10 @@ def cmd_local(self):
         """
         return self.get_cmd()
 
-    def get_args(self, hostname=None, workspace=None):
+    def get_args(self, workspace=None):
         """
         Get the arguments to run the server fixtures.
 
-        @param hostname: hostname of the server
         @param workspace: workspace of the server
         """
         raise NotImplementedError("Concrete class should implement this")
@@ -236,16 +236,28 @@ def _wait_for_go(self, start_interval=0.1, retries_per_interval=3, retry_limit=2
         raise ValueError("Server failed to start up after waiting %s. Giving up!"
                          % str(datetime.now() - start_time))
 
+    def _get_hostname(self):
+        """
+        Get host IP for service to listen on
+        """
+        if self._server_class == 'thread':
+            # serverclass "thread" has special way to do this
+            return get_ephemeral_host() \
+                    if self.random_hostname \
+                    else CONFIG.fixture_hostname
+
+        return '0.0.0.0'
+
     def _get_port(self, default_port):
         """
         Get a random or pseudo-random port based on config.
         """
-        if self._server_class != 'thread':
-            return default_port
+        if self._server_class == 'thread':
+            return get_ephemeral_port(host=self._listen_hostname) \
+                    if self.random_port \
+                    else self._get_pseudo_random_port()
 
-        return (
-            get_ephemeral_port() if self.random_port
-            else self._get_pseudo_random_port())
+        return default_port
 
     def _get_pseudo_random_port(self):
         """

pytest-server-fixtures/pytest_server_fixtures/mongo.py

@@ -93,17 +93,14 @@ def cmd_local(self):
 
     def get_args(self, **kwargs):
         cmd = [
+            '--bind_ip=%s' % self._listen_hostname,
             '--port=%s' % self.port,
             '--nounixsocket',
             '--syncdelay=0',
             '--nojournal',
             '--quiet',
         ]
 
-        if 'hostname' in kwargs:
-            cmd.append('--bind_ip=%s' % kwargs['hostname'])
-        else:
-            cmd.append('--bind_ip=0.0.0.0')
         if 'workspace' in kwargs:
             cmd.append('--dbpath=%s' % str(kwargs['workspace']))
 

pytest-server-fixtures/pytest_server_fixtures/redis.py

@@ -78,6 +78,7 @@ def cmd_local(self):
 
     def get_args(self, **kwargs):
         cmd = [
+            "--bind", self._listen_hostname,
             "--port", str(self.port),
             "--timeout", "0",
             "--loglevel", "notice",
@@ -89,11 +90,6 @@ def get_args(self, **kwargs):
             "--slowlog-max-len", "1024",
         ]
 
-        if 'hostname' in kwargs:
-            cmd += ["--bind", "%s" % kwargs['hostname']]
-        else:
-            cmd += ["--bind", "0.0.0.0"]
-
         return cmd
 
     @property

pytest-server-fixtures/pytest_server_fixtures/rethink.py

@@ -138,16 +138,12 @@ def cmd_local(self):
 
     def get_args(self, **kwargs):
         cmd = [
+            '--bind', self._listen_hostname,
             '--driver-port', str(self.port),
             '--http-port', str(self.http_port),
             '--cluster-port', str(self.cluster_port),
         ]
 
-        if 'hostname' in kwargs:
-            cmd += ['--bind', kwargs['hostname']]
-        else:
-            cmd += ['--bind', 'all']
-
         if 'workspace' in kwargs:
             cmd += ['--directory', str(kwargs['workspace'] / 'db')]
 

pytest-server-fixtures/pytest_server_fixtures/serverclass/__init__.py

@@ -13,7 +13,7 @@ def create_server(server_class, **kwargs):
             env=kwargs["env"],
             workspace=kwargs["workspace"],
             cwd=kwargs["cwd"],
-            random_hostname=kwargs["random_hostname"],
+            listen_hostname=kwargs["listen_hostname"],
         )
 
     if server_class == 'docker':

pytest-server-fixtures/pytest_server_fixtures/serverclass/thread.py

@@ -12,7 +12,7 @@
 from retry import retry
 
 from pytest_server_fixtures import CONFIG
-from pytest_server_fixtures.base import get_ephemeral_host, ProcessReader, ServerNotDead, OSX
+from pytest_server_fixtures.base import ProcessReader
 from .common import ServerClass, is_debug
 
 log = logging.getLogger(__name__)
@@ -64,22 +64,19 @@ def __init__(self,
                  env,
                  workspace,
                  cwd=None,
-                 random_hostname=True):
+                 listen_hostname=None):
         super(ThreadServer, self).__init__(cmd, get_args, env)
 
         self.exit = False
-        self._hostname = get_ephemeral_host() if random_hostname else CONFIG.fixture_hostname
         self._workspace = workspace
         self._cwd = cwd
+        self._hostname = listen_hostname
         self._proc = None
 
     def launch(self):
         log.debug("Launching thread server.")
 
-        run_cmd = [self._cmd] + self._get_args(
-            hostname=self.hostname,
-            workspace=self._workspace,
-        )
+        run_cmd = [self._cmd] + self._get_args(workspace=self._workspace)
 
         debug = is_debug()
 

pytest-server-fixtures/tests/unit/serverclass/test_thread_unit.py

@@ -11,38 +11,12 @@ def test_init(mock_init):
                       sentinel.env,
                       sentinel.workspace,
                       cwd=sentinel.cwd,
-                      random_hostname=False)
+                      listen_hostname=sentinel.listen_hostname)
 
     mock_init.assert_called_with(sentinel.cmd,
                                  sentinel.get_args,
                                  sentinel.env)
+    assert ts._hostname == sentinel.listen_hostname
     assert ts._workspace == sentinel.workspace
     assert ts._cwd == sentinel.cwd
 
-
-@patch('pytest_server_fixtures.serverclass.thread.get_ephemeral_host', return_value=sentinel.host)
-def test_init_with_random_hostname(mock_get_ephemeral_host):
-    ts = ThreadServer(sentinel.cmd,
-                      sentinel.get_args,
-                      sentinel.env,
-                      sentinel.workspace,
-                      cwd=sentinel.cwd,
-                      random_hostname=True)
-
-    mock_get_ephemeral_host.assert_called_once()
-    assert ts.hostname == sentinel.host
-
-
-@patch('pytest_server_fixtures.serverclass.thread.get_ephemeral_host')
-def test_init_without_random_hostname(mock_get_ephemeral_host):
-    ts = ThreadServer(sentinel.cmd,
-                      sentinel.get_args,
-                      sentinel.env,
-                      sentinel.workspace,
-                      cwd=sentinel.cwd,
-                      random_hostname=False)
-
-    mock_get_ephemeral_host.assert_not_called()
-    assert ts.hostname == CONFIG.fixture_hostname
-
-