From 166f6491e9bb8a1ea833adc41796b2d10679656b Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 17 Mar 2026 18:13:22 +0000 Subject: [PATCH 1/3] ci: run strict-no-cover in scripts/test to catch stale pragmas locally PR #2302 passed ./scripts/test (100% coverage) but failed CI when strict-no-cover found lines marked '# pragma: no cover' that the new test actually executed. The claim in CLAUDE.md that ./scripts/test 'matches CI exactly' was false. Adds strict-no-cover as a final step in scripts/test. The tool internally spawns 'uv run coverage json' without --frozen, which rewrites uv.lock on machines with registry overrides; UV_FROZEN=1 propagates to that subprocess. Also converts session.py:426 from 'no cover' to 'lax no cover'. The except-handler during connection cleanup is a genuine race (stream may or may not already be closed depending on timing), so it is nondeterministically covered. strict-no-cover would have flagged it intermittently on high-core machines. CLAUDE.md now documents the fast targeted path (single test file + strict-no-cover, ~4s, no false positives on partial runs), the three pragma types, and the known coverage.py ->exit arc quirk with nested async-with that only the CI matrix catches. --- CLAUDE.md | 25 ++++++++++++++++++++++--- scripts/test | 3 +++ src/mcp/shared/session.py | 2 +- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 98bd45115..290d1109b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,18 +29,37 @@ This document contains critical information about working with this codebase. Fo - IMPORTANT: The `tests/client/test_client.py` is the most well designed test file. Follow its patterns. - IMPORTANT: Be minimal, and focus on E2E tests: Use the `mcp.client.Client` whenever possible. - Coverage: CI requires 100% (`fail_under = 100`, `branch = true`). - - Full check: `./scripts/test` (~20s, matches CI exactly) - - Targeted check while iterating: + - Full check: `./scripts/test` (~23s). Runs coverage + `strict-no-cover` on the + default Python. Not identical to CI: CI also runs 3.10–3.14 × {ubuntu, windows}, + and some branch-coverage quirks only surface on specific matrix entries. + - Targeted check while iterating (~4s, deterministic): ```bash uv run --frozen coverage erase uv run --frozen coverage run -m pytest tests/path/test_foo.py uv run --frozen coverage combine uv run --frozen coverage report --include='src/mcp/path/foo.py' --fail-under=0 + UV_FROZEN=1 uv run --frozen strict-no-cover ``` Partial runs can't hit 100% (coverage tracks `tests/` too), so `--fail-under=0` - and `--include` scope the report to what you actually changed. + and `--include` scope the report. `strict-no-cover` has no false positives on + partial runs — if your new test executes a line marked `# pragma: no cover`, + even a single-file run catches it. + - `strict-no-cover` can occasionally flag subprocess-runner functions in `tests/` + on high-core machines (`-n auto` → racy subprocess coverage). If the flagged + lines are inside a `tests/…/run_*server*()` body and unrelated to your change, + re-run or convert that pragma to `lax no cover`. Flags in `src/` are real. + - Coverage pragmas: + - `# pragma: no cover` — line is never executed. CI's `strict-no-cover` fails if + it IS executed. When your test starts covering such a line, remove the pragma. + - `# pragma: lax no cover` — excluded from coverage but not checked by + `strict-no-cover`. Use for lines covered on some platforms/versions but not + others, or nondeterministically (races, subprocess coverage). + - `# pragma: no branch` — excludes branch arcs only. coverage.py misreports the + `->exit` arc for nested `async with` on Python 3.11+ (worse on 3.14/Windows). + There is no local detector; when CI reports `X->exit` missing on a test line, + add this pragma to line X. - Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead: - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()` diff --git a/scripts/test b/scripts/test index ee1259b59..dc43f351d 100755 --- a/scripts/test +++ b/scripts/test @@ -6,3 +6,6 @@ uv run --frozen coverage erase uv run --frozen coverage run -m pytest -n auto $@ uv run --frozen coverage combine uv run --frozen coverage report +# strict-no-cover spawns `uv run coverage json` internally without --frozen; +# UV_FROZEN=1 propagates to that subprocess so it doesn't touch uv.lock. +UV_FROZEN=1 uv run --frozen strict-no-cover diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 9364abb73..42ff12ba8 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -423,7 +423,7 @@ async def _receive_loop(self) -> None: try: await stream.send(JSONRPCError(jsonrpc="2.0", id=id, error=error)) await stream.aclose() - except Exception: # pragma: no cover + except Exception: # pragma: lax no cover # Stream might already be closed pass self._response_streams.clear() From 6165b2eb575c21c561c6a9db5f5f969bfea6b2b4 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 17 Mar 2026 18:21:49 +0000 Subject: [PATCH 2/3] revert: undo session.py pragma change (speculative, bad evidence) The session.py:426 flag appeared exactly once across 10 runs, and that one run was the only one with a confounding test edit in place. CI has never flagged this line; 8 clean local runs never flagged it either. The original pragma was correct. --- src/mcp/shared/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 42ff12ba8..9364abb73 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -423,7 +423,7 @@ async def _receive_loop(self) -> None: try: await stream.send(JSONRPCError(jsonrpc="2.0", id=id, error=error)) await stream.aclose() - except Exception: # pragma: lax no cover + except Exception: # pragma: no cover # Stream might already be closed pass self._response_streams.clear() From daf837c8602c855906ad6411529a38b946b9e468 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 17 Mar 2026 19:21:44 +0000 Subject: [PATCH 3/3] docs: tighten coverage-pragma guidance Drop the subprocess-flake bullet and the 'add this pragma' prescription for no-branch. Scope lax-no-cover to platform-specific coverage only. --- CLAUDE.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 290d1109b..2eee085e1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,20 +46,14 @@ This document contains critical information about working with this codebase. Fo and `--include` scope the report. `strict-no-cover` has no false positives on partial runs — if your new test executes a line marked `# pragma: no cover`, even a single-file run catches it. - - `strict-no-cover` can occasionally flag subprocess-runner functions in `tests/` - on high-core machines (`-n auto` → racy subprocess coverage). If the flagged - lines are inside a `tests/…/run_*server*()` body and unrelated to your change, - re-run or convert that pragma to `lax no cover`. Flags in `src/` are real. - Coverage pragmas: - `# pragma: no cover` — line is never executed. CI's `strict-no-cover` fails if it IS executed. When your test starts covering such a line, remove the pragma. - `# pragma: lax no cover` — excluded from coverage but not checked by `strict-no-cover`. Use for lines covered on some platforms/versions but not - others, or nondeterministically (races, subprocess coverage). + others. - `# pragma: no branch` — excludes branch arcs only. coverage.py misreports the `->exit` arc for nested `async with` on Python 3.11+ (worse on 3.14/Windows). - There is no local detector; when CI reports `X->exit` missing on a test line, - add this pragma to line X. - Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead: - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()`