Add tests for server ready state fix

Author: HayaoSuzuki

pytest_httpserver/blocking_httpserver.py

@@ -46,6 +46,8 @@ class BlockingHTTPServer(HTTPServerBase):
 
     :param timeout: waiting time in seconds for matching and responding to an incoming request.
         manager
+    :param startup_timeout: maximum time in seconds to wait for server readiness.
+        Set to ``None`` to disable readiness waiting.
 
     .. py:attribute:: no_handler_status_code
 
@@ -63,8 +65,10 @@ def __init__(
         port: int = DEFAULT_LISTEN_PORT,
         ssl_context: SSLContext | None = None,
         timeout: int = 30,
+        *,
+        startup_timeout: float | None = 10.0,
     ) -> None:
-        super().__init__(host, port, ssl_context)
+        super().__init__(host, port, ssl_context, startup_timeout=startup_timeout)
         self.timeout = timeout
         self.request_queue: Queue[Request] = Queue()
         self.request_handlers: dict[Request, Queue[BlockingRequestHandler]] = {}

pytest_httpserver/httpserver.py

@@ -8,6 +8,7 @@
 import threading
 import time
 import urllib.parse
+import warnings
 from collections import defaultdict
 from collections.abc import Callable
 from collections.abc import Generator
@@ -615,6 +616,8 @@ class HTTPServerBase(abc.ABC):  # pylint: disable=too-many-instance-attributes
     :param port: the TCP port where the server will listen
     :param ssl_context: the ssl context object to use for https connections
     :param threaded: whether to handle concurrent requests in separate threads
+    :param startup_timeout: maximum time in seconds to wait for server readiness.
+        Set to ``None`` to disable readiness waiting.
 
     .. py:attribute:: log
 
@@ -638,6 +641,7 @@ def __init__(
         ssl_context: SSLContext | None = None,
         *,
         threaded: bool = False,
+        startup_timeout: float | None = 10.0,
     ) -> None:
         """
         Initializes the instance.
@@ -652,6 +656,7 @@ def __init__(
         self.log: list[tuple[Request, Response]] = []
         self.ssl_context = ssl_context
         self.threaded = threaded
+        self.startup_timeout = startup_timeout
         self.no_handler_status_code = 500
         self._server_ready_event: threading.Event = threading.Event()
 
@@ -735,8 +740,10 @@ def thread_target(self) -> None:
 
         This should not be called directly, but can be overridden to tailor it to your needs.
 
-        If overriding, you must call ``self._server_ready_event.set()`` before starting
-        to serve requests, otherwise :py:meth:`start` will raise an error after timeout.
+        If overriding, you should call ``self._server_ready_event.set()`` before starting
+        to serve requests. If the event is not set within the timeout, :py:meth:`start`
+        will emit a warning if the thread is still alive; if the thread dies during
+        startup, :py:meth:`start` raises an error.
         """
         assert self.server is not None
         self._server_ready_event.set()
@@ -779,22 +786,33 @@ def start(self) -> None:
 
         self.port = self.server.port  # Update port (needed if `port` was set to 0)
         # Explicitly make the new thread daemonic to avoid shutdown issues
-        self._server_ready_event.clear()
+        # Create a new event for each startup to prevent stale threads from
+        # signaling readiness for a subsequent start() attempt.
+        self._server_ready_event = threading.Event()
         self.server_thread = threading.Thread(target=self.thread_target, daemon=True)
         self.server_thread.start()
-        if not self._server_ready_event.wait(timeout=10):
-            # Clean up the server before raising.
-            # Use server_close() instead of shutdown() to avoid deadlock
-            # if serve_forever() was never called.
-            self.server.server_close()
-            self.server_thread.join(timeout=5)
-            self.server = None
-            self.server_thread = None
-            raise HTTPServerError(
-                "Server did not start within timeout. "
-                "If you override thread_target(), ensure it calls "
-                "self._server_ready_event.set() before serving."
-            )
+        if self.startup_timeout is not None and not self._server_ready_event.wait(timeout=self.startup_timeout):
+            # Event was not set within timeout.
+            # Check if thread is still alive (custom thread_target may not set the event)
+            if self.server_thread.is_alive():
+                # Server thread is running, assume it's working (backward compatibility)
+                warnings.warn(
+                    "Server thread is running but ready event was not set. "
+                    "If you override thread_target(), call self._server_ready_event.set() "
+                    "before serving to ensure reliable startup.",
+                    stacklevel=2,
+                )
+            else:
+                # Thread died, clean up and raise
+                self.server.server_close()
+                self.server_thread.join(timeout=5)
+                self.server = None
+                self.server_thread = None
+                raise HTTPServerError(
+                    "Server thread died during startup. "
+                    "If you override thread_target(), ensure it calls "
+                    "self._server_ready_event.set() before serving."
+                )
 
     def stop(self) -> None:
         """
@@ -956,6 +974,8 @@ class HTTPServer(HTTPServerBase):  # pylint: disable=too-many-instance-attribute
         manager
 
     :param threaded: whether to handle concurrent requests in separate threads
+    :param startup_timeout: maximum time in seconds to wait for server readiness.
+        Set to ``None`` to disable readiness waiting.
 
     .. py:attribute:: no_handler_status_code
 
@@ -975,11 +995,18 @@ def __init__(
         default_waiting_settings: WaitingSettings | None = None,
         *,
         threaded: bool = False,
+        startup_timeout: float | None = 10.0,
     ) -> None:
         """
         Initializes the instance.
         """
-        super().__init__(host, port, ssl_context, threaded=threaded)
+        super().__init__(
+            host,
+            port,
+            ssl_context,
+            threaded=threaded,
+            startup_timeout=startup_timeout,
+        )
 
         self.ordered_handlers: list[RequestHandler] = []
         self.oneshot_handlers = RequestHandlerList()

tests/test_release.py

@@ -232,6 +232,7 @@ def test_sdist_contents(build: Build, version: str):
             "test_querymatcher.py",
             "test_querystring.py",
             "test_release.py",
+            "test_server_startup.py",
             "test_ssl.py",
             "test_thread_type.py",
             "test_threaded.py",

tests/test_server_startup.py

@@ -0,0 +1,114 @@
+from __future__ import annotations
+
+import contextlib
+import socket
+import time
+
+import pytest
+
+from pytest_httpserver import HTTPServer
+
+
+def test_server_ready_immediately_after_start():
+    """Test that the server accepts connections immediately after start() returns."""
+    server = HTTPServer(host="localhost", port=0)
+    server.expect_request("/").respond_with_data("ok")
+    server.start()
+    try:
+        # Attempt to connect immediately - should not fail
+        sock = socket.create_connection((server.host, server.port), timeout=1)
+        sock.close()
+    finally:
+        server.stop()
+
+
+def test_server_ready_under_load():
+    """Test that the server is ready even when started multiple times in succession."""
+    for _ in range(10):
+        server = HTTPServer(host="localhost", port=0)
+        server.expect_request("/").respond_with_data("ok")
+        server.start()
+        try:
+            sock = socket.create_connection((server.host, server.port), timeout=1)
+            sock.close()
+        finally:
+            server.stop()
+
+
+class SlowStartServer(HTTPServer):
+    """A server subclass that simulates slow startup."""
+
+    def thread_target(self):
+        time.sleep(0.5)  # Simulate slow initialization
+        self._server_ready_event.set()
+        assert self.server is not None
+        self.server.serve_forever()
+
+
+class NoReadyEventServer(HTTPServer):
+    """A server subclass that never signals readiness."""
+
+    def thread_target(self):
+        assert self.server is not None
+        self.server.serve_forever()
+
+
+def test_slow_start_server_waits_for_ready():
+    """Test that start() waits for slow thread_target implementations."""
+    server = SlowStartServer(host="localhost", port=0)
+    server.expect_request("/").respond_with_data("ok")
+
+    start_time = time.time()
+    server.start()
+    elapsed = time.time() - start_time
+
+    try:
+        # Should have waited at least 0.5 seconds
+        assert elapsed >= 0.5
+        # Server should be ready
+        sock = socket.create_connection((server.host, server.port), timeout=1)
+        sock.close()
+    finally:
+        server.stop()
+
+
+def test_new_event_created_for_each_start():
+    """Test that a new event is created for each start() to isolate retries."""
+    server = HTTPServer(host="localhost", port=0)
+    server.expect_request("/").respond_with_data("ok")
+
+    original_event = server._server_ready_event  # noqa: SLF001
+
+    server.start()
+    first_start_event = server._server_ready_event  # noqa: SLF001
+    server.stop()
+
+    server.start()
+    second_start_event = server._server_ready_event  # noqa: SLF001
+    server.stop()
+
+    # Each start() should create a new event
+    assert first_start_event is not original_event
+    assert second_start_event is not first_start_event
+
+
+def test_warns_when_ready_event_not_set():
+    """Test that a warning is emitted when the ready event is never set."""
+    server = NoReadyEventServer(host="localhost", port=0, startup_timeout=0.0)
+    server.expect_request("/").respond_with_data("ok")
+
+    with pytest.warns(UserWarning, match="ready event was not set"):
+        server.start()
+
+    try:
+        deadline = time.time() + 1
+        while time.time() < deadline:
+            with contextlib.suppress(OSError):
+                sock = socket.create_connection((server.host, server.port), timeout=0.1)
+                sock.close()
+                break
+            time.sleep(0.01)
+        else:
+            raise AssertionError("Server did not accept connections within 1 second")
+    finally:
+        server.stop()