diff --git a/docs/development/tests.mdx b/docs/development/tests.mdx index bbdbd972a..a5674ade6 100644 --- a/docs/development/tests.mdx +++ b/docs/development/tests.mdx @@ -26,7 +26,7 @@ uv run pytest --cov=fastmcp uv run pytest -m "not integration" # Skip tests that spawn processes -uv run pytest -m "not integration and not client_process" +uv run pytest -m "not integration and not client_process and not subprocess_heavy" ``` Tests should complete in under 1 second unless marked as integration tests. This speed encourages running them frequently, catching issues early. @@ -61,6 +61,40 @@ async def test_stdio_transport(): assert result.content[0].text == "test" ``` +A third marker, `subprocess_heavy`, exists specifically for Windows CI stability. See [Windows CI and Test Parallelism](#windows-ci-and-test-parallelism) below for when to use it and why it exists. + +### Windows CI and Test Parallelism + +Windows CI ran the unit suite serially for months. [#2715](https://github.com/PrefectHQ/fastmcp/pull/2715) tried enabling `pytest-xdist` parallelism there in December 2025; [#2726](https://github.com/PrefectHQ/fastmcp/pull/2726) reverted it the next day because "Windows tests continue to fail with intermittent worker crashes." [#4554](https://github.com/PrefectHQ/fastmcp/pull/4554) re-enabled it after removing most of the subprocess pressure that caused those crashes, taking the Windows unit step from roughly 460s to 175s. + +That pressure came from three sources, all addressed by #4554: most HTTP tests moved in-process via `asgi_client` instead of binding real sockets, stdio lifecycle tests spawn a minimal stdlib responder (`tests/client/minimal_stdio_server.py`, ~0.03s to start) instead of a subprocess that runs `import fastmcp` (~0.7s), and roughly 80 real `sleep()` calls became deterministic waits on the condition each test actually cared about. Fewer, cheaper subprocesses competing under parallel workers left fewer chances for a worker to die. + +#### The `subprocess_heavy` marker + +One class of test still spawns a full Python interpreter that imports FastMCP — checking that a bare install doesn't need optional dependencies, or that a decorator works from a fresh process. Each spawn pays a full interpreter's startup and memory footprint, and a 2-core Windows runner already running 2 xdist workers has little headroom left to absorb that. These tests carry `@pytest.mark.subprocess_heavy` and run in the existing serial `client_process` CI step instead of alongside the parallel workers — `.github/actions/run-pytest/action.yml` routes `client_process or subprocess_heavy` to that step (`MAX_PROCS=0`) and excludes both markers from the parallel unit step. + +If a test runs `subprocess.run([sys.executable, "-c", ...])`, or otherwise starts a fresh interpreter that imports `fastmcp`, mark it `subprocess_heavy`. A subprocess that runs a minimal stdlib script with no FastMCP import doesn't need the marker — it's the interpreter startup and import that's expensive, not the subprocess itself. + +#### This is a mitigation, not a proof + +There is no root-cause diagnosis behind this fix, only a plausible one. During validation, one Windows run genuinely crashed a worker on `test_fastmcp_imports_without_legacy_httpx` — a fresh-interpreter test — with pytest-xdist reporting `worker 'gw1' crashed while running '...'` after execnet's channel saw `ConnectionResetError: [WinError 10054]`. Nothing in that log says *why* the worker died: memory exhaustion, handle exhaustion, and some Windows-specific `subprocess`/`execnet` interaction are all still consistent with what was observed. Marking the fresh-interpreter tests `subprocess_heavy` made the crash stop recurring, but "it stopped" is not the same as "we know why." + +Treat the next Windows worker crash as a test of this diagnosis. **If it lands on a test that is not a fresh-interpreter spawner, the `subprocess_heavy` theory was wrong** — the real problem is subprocess-under-xdist on Windows more generally, and isolating one marker's worth of tests was never going to fix that. The fallback is one conditional back in `run-pytest/action.yml`, restoring the pre-#4554 behavior: + +```bash +PARALLEL_FLAGS="" +if [ "$MAX_PROCS" != "0" ] && [ "${{ runner.os }}" != "Windows" ]; then + PARALLEL_FLAGS="--numprocesses auto --maxprocesses $MAX_PROCS --dist worksteal" +fi +``` + +#### Two traps that aren't Windows-specific + +Two test-authoring bugs surfaced while validating this change. Neither is about Windows or parallelism, but both are worth watching for anywhere a real `sleep()` gets replaced with a wait: + +- **Match the wait condition to the assertion.** A test waited for "any callback fired," then asserted that a `completed` callback existed. That races, because an earlier `working` notification satisfies the wait before the `completed` one arrives. A deterministic wait is only as good as the condition it waits on — wait for the thing you actually assert. +- **Don't assert on incidental timing.** A crash-recovery test asserted "at least one concurrent request fails" while a subprocess restarts, which quietly depended on the restart being slow. Once restart got faster, recovery could beat every in-flight request and the test started failing because the behavior *improved*. Assert the invariant instead: no hang, and no result served by the dead process. + ## Writing Tests