remote/datasette
1
0
Fork 0
mirror of https://github.com/simonw/datasette.git synced 2026-09-09 01:54:15 +02:00
Code Issues Projects Releases Packages Wiki Activity

Move the serve-subprocess test plumbing into a conftest fixture

Browse source 
The two new tests span a datasette serve subprocess by hand: each found a
free port with a copy of test_playwright.py's find_free_port, built its own
subprocess.Popen call, polled with its own 15 second deadline loop, and
tore the process down in its own finally block. That is the third and
fourth hand-rolled copy of plumbing conftest.py already owns for
ds_localhost_http_server and ds_unix_domain_socket_server.

Move find_free_port into conftest.py and add a serve_with_plugins factory
fixture that writes plugin sources to a temporary --plugins-dir, takes a
free port, waits for the server to answer, and terminates every process it
started when the test ends. wait_until_responds() grows an optional
process argument so a server that dies during startup fails immediately
with its captured output instead of waiting out the timeout, and now
catches httpx.TransportError rather than only httpx.ConnectError - a
superclass, so existing callers are unaffected.

Two fixes beyond the deduplication:

The marker test polls until its flag flips, which meant it would also have
passed if the startup hook were re-run on the serving loop by the
first-request fallback - the exact bug it exists to catch. That cannot
happen while invoke_startup() is idempotent, but nothing said so. The
plugin now counts startup calls and the test asserts it ran exactly once,
so removing that guard fails the test loudly instead of quietly turning it
into a no-op.

test_startup_error_fails_fast_before_port_binds passes on unmodified main,
where startup already ran ahead of uvicorn.run(), so it is a
characterization test rather than a regression test for this commit; its
docstring now says so. Its loop re-checking that nothing was listening ran
about one iteration before the process exited, and could not distinguish a
pre-bind failure from a port nothing ever touched, so it is replaced by a
single check with a comment about what it does and does not prove.

Verified the red side is preserved: the marker test still fails on
unmodified main, now in 3.7s rather than 15.2s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Alex Garcia 2026-08-03 16:52:48 -07:00
commit ba3756eb54
2 changed files with 136 additions and 112 deletions
Download patch file Download diff file Expand all files Collapse all files

80
tests/conftest.py
View file

@ -2,6 +2,7 @@ import importlib.metadata
import os
import pathlib
import re
import socket
import subprocess
import sys
import tempfile
@ -32,17 +33,31 @@ UNDOCUMENTED_PERMISSIONS = {
}
def wait_until_responds(url, timeout=5.0, client=httpx, **kwargs):
def wait_until_responds(url, timeout=5.0, client=httpx, process=None, **kwargs):
start = time.time()
while time.time() - start < timeout:
# If the server died there is no point waiting out the timeout - fail
# now, with its output, instead of after `timeout` seconds of silence
if process is not None and process.poll() is not None:
raise AssertionError(
"Server exited early with returncode {}\n{}".format(
process.returncode, process.stdout.read().decode("utf-8")
)
)
try:
client.get(url, **kwargs)
return
except httpx.ConnectError:
except httpx.TransportError:
time.sleep(0.1)
raise AssertionError(f"Timed out waiting for {url} to respond")
def find_free_port():
with socket.socket() as sock:
sock.bind(("127.0.0.1", 0))
return sock.getsockname()[1]
@pytest.fixture
def bare_ds():
"""
@ -301,6 +316,67 @@ def ds_unix_domain_socket_server(tmp_path_factory):
pass
@pytest.fixture
def serve_with_plugins(tmp_path):
"""Factory fixture for starting ``datasette serve`` in a subprocess with
plugins written to a temporary ``--plugins-dir``.
Unlike ``ds_localhost_http_server`` this is function-scoped and takes a
fresh port each time, because each test needs its own plugins. Call it as::
proc, port = serve_with_plugins({"my_plugin": PLUGIN_SOURCE})
``plugins`` maps module name to Python source. Pass
``wait_for_startup=False`` when the server is expected to fail during
startup rather than begin serving. Extra CLI arguments are passed through.
Every process started is terminated when the test ends.
"""
processes = []
def start(plugins, *extra_args, wait_for_startup=True):
plugins_dir = tmp_path / "plugins"
plugins_dir.mkdir(exist_ok=True)
for module_name, source in plugins.items():
(plugins_dir / "{}.py".format(module_name)).write_text(source, "utf-8")
port = find_free_port()
proc = subprocess.Popen(
[
sys.executable,
"-m",
"datasette",
"--memory",
"--plugins-dir",
str(plugins_dir),
"-h",
"127.0.0.1",
"-p",
str(port),
*extra_args,
],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
# Avoid FileNotFoundError: [Errno 2] No such file or directory:
cwd=tempfile.gettempdir(),
)
processes.append(proc)
if wait_for_startup:
wait_until_responds(
"http://127.0.0.1:{}/-/versions.json".format(port), process=proc
)
return proc, port
yield start
for proc in processes:
if proc.poll() is None:
proc.terminate()
try:
proc.wait(timeout=5)
except subprocess.TimeoutExpired:
proc.kill()
proc.wait()
# Import fixtures from fixtures.py to make them available
from .fixtures import ( # noqa: F401
TEMP_PLUGIN_SECRET_FILE,

166
tests/test_cli_serve_server.py
View file

@ -1,7 +1,4 @@
import socket
import subprocess
import sys
import tempfile
import time
import httpx
@ -34,12 +31,6 @@ def test_serve_unix_domain_socket(ds_unix_domain_socket_server):
}.items() <= response.json().items()
def _find_free_port():
with socket.socket() as sock:
sock.bind(("127.0.0.1", 0))
return sock.getsockname()[1]
# Shaped after datasette-litestream's (sync) startup hook, which schedules a
# background task with asyncio.get_running_loop().create_task(...):
# https://github.com/simonw/datasette-litestream/blob/main/datasette_litestream/__init__.py
@ -47,7 +38,7 @@ def _find_free_port():
# same event loop that goes on to serve requests - if it runs on a throwaway
# loop that gets closed straight after (as on unmodified main), the task is
# scheduled but never gets a turn before the loop is torn down.
MARKER_TASK_PLUGIN = '''
MARKER_TASK_PLUGIN = """
import asyncio
from datasette import hookimpl
from datasette.utils.asgi import Response
@ -55,6 +46,8 @@ from datasette.utils.asgi import Response
@hookimpl
def startup(datasette):
datasette._startup_calls = getattr(datasette, "_startup_calls", 0) + 1
async def _mark():
# The await is essential to the regression: a task with no
# internal await point can complete during the brief window
@ -74,14 +67,17 @@ def startup(datasette):
def register_routes():
async def marker_status(datasette):
return Response.json(
{"marker_task_ran": getattr(datasette, "_marker_task_ran", False)}
{
"marker_task_ran": getattr(datasette, "_marker_task_ran", False),
"startup_calls": getattr(datasette, "_startup_calls", 0),
}
)
return [(r"^/-/marker-task-ran$", marker_status)]
'''
"""
STARTUP_ERROR_PLUGIN = '''
STARTUP_ERROR_PLUGIN = """
from datasette import hookimpl
from datasette.utils import StartupError
@ -89,11 +85,11 @@ from datasette.utils import StartupError
@hookimpl
def startup(datasette):
raise StartupError("boom from plugin")
'''
"""
@pytest.mark.serial
def test_startup_hook_background_task_runs_on_serving_loop(tmp_path):
def test_startup_hook_background_task_runs_on_serving_loop(serve_with_plugins):
"""
Litestream-shaped regression test: a startup hook that does
asyncio.get_running_loop().create_task(...) must have that task
@ -103,109 +99,61 @@ def test_startup_hook_background_task_runs_on_serving_loop(tmp_path):
invoke_startup() runs on a throwaway loop that is closed before
uvicorn opens its own loop to serve.
"""
plugins_dir = tmp_path / "plugins"
plugins_dir.mkdir()
(plugins_dir / "marker_task_plugin.py").write_text(MARKER_TASK_PLUGIN, "utf-8")
port = _find_free_port()
ds_proc = subprocess.Popen(
[
sys.executable,
"-m",
"datasette",
"--memory",
"--plugins-dir",
str(plugins_dir),
"-p",
str(port),
],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
cwd=tempfile.gettempdir(),
_, port = serve_with_plugins({"marker_task_plugin": MARKER_TASK_PLUGIN})
# The fixture has already waited for the server to answer requests. The
# marker task deliberately awaits before setting its flag, so poll for a
# moment rather than assuming it landed before the first request arrived.
deadline = time.time() + 3.0
payload = {}
while time.time() < deadline:
payload = httpx.get(
f"http://127.0.0.1:{port}/-/marker-task-ran", timeout=1.0
).json()
if payload["marker_task_ran"]:
break
time.sleep(0.05)
assert payload.get("marker_task_ran"), (
"The startup hook's asyncio.create_task(...) never ran - "
"invoke_startup() and the server are not sharing an event loop"
)
try:
url = f"http://localhost:{port}/-/marker-task-ran"
deadline = time.time() + 15.0
marker_task_ran = False
while time.time() < deadline:
if ds_proc.poll() is not None:
raise AssertionError(
"datasette serve exited early\n"
+ ds_proc.stdout.read().decode("utf-8")
)
try:
response = httpx.get(url, timeout=1.0)
except httpx.TransportError:
time.sleep(0.1)
continue
if response.status_code == 200 and response.json().get(
"marker_task_ran"
):
marker_task_ran = True
break
time.sleep(0.1)
assert marker_task_ran, (
"The startup hook's asyncio.create_task(...) never ran - "
"invoke_startup() and the server are not sharing an event loop"
# Polling above means this test would also pass if the startup hook were
# re-run on the serving loop by the first-request fallback - which would
# hide exactly the bug being tested. invoke_startup() is idempotent today
# so that cannot happen; assert it explicitly so that if the idempotency
# guard is ever removed this test fails loudly instead of silently
# becoming a no-op.
assert payload["startup_calls"] == 1, (
"startup hook ran {} times - the marker may have been set by a "
"re-run on the serving loop rather than by the original task".format(
payload["startup_calls"]
)
finally:
ds_proc.terminate()
try:
ds_proc.wait(timeout=5)
except subprocess.TimeoutExpired:
ds_proc.kill()
ds_proc.wait()
)
@pytest.mark.serial
def test_startup_error_fails_fast_before_port_binds(tmp_path):
def test_startup_error_fails_fast_before_port_binds(serve_with_plugins):
"""
A "startup" plugin hook that raises StartupError must fail fast: print
the message, exit non-zero, and never accept a connection on the port -
the failure must happen before uvicorn.Server binds the socket.
Note this is a characterization test, not a regression test: it also
passes on unmodified main, where startup already ran ahead of
uvicorn.run(). It earns its keep once startup moves into the ASGI
lifespan, where fail-fast is genuinely at risk.
"""
plugins_dir = tmp_path / "plugins"
plugins_dir.mkdir()
(plugins_dir / "startup_error_plugin.py").write_text(
STARTUP_ERROR_PLUGIN, "utf-8"
proc, port = serve_with_plugins(
{"startup_error_plugin": STARTUP_ERROR_PLUGIN}, wait_for_startup=False
)
stdout, _ = proc.communicate(timeout=15)
output = stdout.decode("utf-8")
assert proc.returncode not in (0, None), output
assert "boom from plugin" in output, output
port = _find_free_port()
ds_proc = subprocess.Popen(
[
sys.executable,
"-m",
"datasette",
"--memory",
"--plugins-dir",
str(plugins_dir),
"-p",
str(port),
],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
cwd=tempfile.gettempdir(),
)
try:
deadline = time.time() + 15.0
# While the process is still alive (it should crash almost
# immediately) repeatedly confirm nothing is listening on the port
while ds_proc.poll() is None and time.time() < deadline:
with pytest.raises(OSError):
with socket.create_connection(("127.0.0.1", port), timeout=0.2):
pass
time.sleep(0.05)
stdout, _ = ds_proc.communicate(timeout=5)
output = stdout.decode("utf-8")
assert ds_proc.returncode not in (0, None), output
assert "boom from plugin" in output, output
# And confirm it never accepted a connection even now it has exited
with pytest.raises(OSError):
with socket.create_connection(("127.0.0.1", port), timeout=0.2):
pass
finally:
if ds_proc.poll() is None:
ds_proc.kill()
ds_proc.wait()
# Nothing is listening on the port now the process has exited. This
# confirms the socket was not left bound; on its own it cannot prove the
# failure preceded the bind, since a port nothing ever touched also
# refuses connections.
with pytest.raises(OSError):
with socket.create_connection(("127.0.0.1", port), timeout=0.2):
pass