From 7db8654dec3a4938aa03366b77fc699437d61521 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Fri, 6 Mar 2026 15:35:44 +0000 Subject: [PATCH] fix: make local coverage runs reliable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes for local coverage reliability: 1. scripts/test: add `coverage erase` before `coverage run`. The config implicitly enables parallel=True (via concurrency=['multiprocessing']), so each run produces ~70 .coverage.* files. If a prior run aborted before `coverage combine` (e.g. flaky test + set -e), those stale files get picked up. Erase guarantees a clean slate. 2. CLAUDE.md: the previous instruction (`pytest -x` to verify coverage) never worked — pytest-cov isn't installed and coverage isn't wired into pytest addopts. Replace with the actual commands, plus a targeted-check recipe for iterating on a single module. Notes that partial runs can't hit 100% because source includes tests/. 3. test_desktop: was flaky under -n auto when it landed on an xdist worker that hadn't yet imported jsonschema. The class-level Path.iterdir monkeypatch broke jsonschema_specifications' import-time schema discovery (which walks a directory). Use a real tmp_path Desktop directory instead of patching iterdir. Also adds `coverage erase` to the CI workflow so the command sequence there stays copy-pasteable into a local shell. --- .github/workflows/shared.yml | 1 + CLAUDE.md | 16 +++++++++++++--- scripts/test | 1 + tests/test_examples.py | 27 ++++++++++++--------------- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/.github/workflows/shared.yml b/.github/workflows/shared.yml index 72e328b54..efb45c889 100644 --- a/.github/workflows/shared.yml +++ b/.github/workflows/shared.yml @@ -70,6 +70,7 @@ jobs: - name: Run pytest with coverage shell: bash run: | + uv run --frozen --no-sync coverage erase uv run --frozen --no-sync coverage run -m pytest -n auto uv run --frozen --no-sync coverage combine uv run --frozen --no-sync coverage report diff --git a/CLAUDE.md b/CLAUDE.md index e48ce6e70..98bd45115 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,9 +28,19 @@ This document contains critical information about working with this codebase. Fo - Bug fixes require regression tests - 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. - - IMPORTANT: Before pushing, verify 100% branch coverage on changed files by running - `uv run --frozen pytest -x` (coverage is configured in `pyproject.toml` with `fail_under = 100` - and `branch = true`). If any branch is uncovered, add a test for it before pushing. + - Coverage: CI requires 100% (`fail_under = 100`, `branch = true`). + - Full check: `./scripts/test` (~20s, matches CI exactly) + - Targeted check while iterating: + + ```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 + ``` + + Partial runs can't hit 100% (coverage tracks `tests/` too), so `--fail-under=0` + and `--include` scope the report to what you actually changed. - 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 0d08e47b1..ee1259b59 100755 --- a/scripts/test +++ b/scripts/test @@ -2,6 +2,7 @@ set -ex +uv run --frozen coverage erase uv run --frozen coverage run -m pytest -n auto $@ uv run --frozen coverage combine uv run --frozen coverage report diff --git a/tests/test_examples.py b/tests/test_examples.py index aa9de0957..3af82f04c 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -5,7 +5,6 @@ # pyright: reportUnknownArgumentType=false # pyright: reportUnknownMemberType=false -import sys from pathlib import Path import pytest @@ -65,12 +64,17 @@ async def test_direct_call_tool_result_return(): @pytest.mark.anyio -async def test_desktop(monkeypatch: pytest.MonkeyPatch): +async def test_desktop(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): """Test the desktop server""" - # Mock desktop directory listing - mock_files = [Path("/fake/path/file1.txt"), Path("/fake/path/file2.txt")] - monkeypatch.setattr(Path, "iterdir", lambda self: mock_files) # type: ignore[reportUnknownArgumentType] - monkeypatch.setattr(Path, "home", lambda: Path("/fake/home")) + # Build a real Desktop directory under tmp_path rather than patching + # Path.iterdir — a class-level patch breaks jsonschema_specifications' + # import-time schema discovery when this test happens to be the first + # tool call in an xdist worker. + desktop = tmp_path / "Desktop" + desktop.mkdir() + (desktop / "file1.txt").touch() + (desktop / "file2.txt").touch() + monkeypatch.setattr(Path, "home", lambda: tmp_path) from examples.mcpserver.desktop import mcp @@ -85,15 +89,8 @@ async def test_desktop(monkeypatch: pytest.MonkeyPatch): content = result.contents[0] assert isinstance(content, TextResourceContents) assert isinstance(content.text, str) - if sys.platform == "win32": # pragma: no cover - file_1 = "/fake/path/file1.txt".replace("/", "\\\\") # might be a bug - file_2 = "/fake/path/file2.txt".replace("/", "\\\\") # might be a bug - assert file_1 in content.text - assert file_2 in content.text - # might be a bug, but the test is passing - else: # pragma: lax no cover - assert "/fake/path/file1.txt" in content.text - assert "/fake/path/file2.txt" in content.text + assert "file1.txt" in content.text + assert "file2.txt" in content.text # TODO(v2): Change back to README.md when v2 is released