fix: make local coverage runs reliable (#2236)
This commit is contained in:
parent
eaf971cf25
commit
7ba41dcfae
4 changed files with 27 additions and 18 deletions
1
.github/workflows/shared.yml
vendored
1
.github/workflows/shared.yml
vendored
|
|
@ -70,6 +70,7 @@ jobs:
|
||||||
- name: Run pytest with coverage
|
- name: Run pytest with coverage
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
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 run -m pytest -n auto
|
||||||
uv run --frozen --no-sync coverage combine
|
uv run --frozen --no-sync coverage combine
|
||||||
uv run --frozen --no-sync coverage report
|
uv run --frozen --no-sync coverage report
|
||||||
|
|
|
||||||
16
CLAUDE.md
16
CLAUDE.md
|
|
@ -28,9 +28,19 @@ This document contains critical information about working with this codebase. Fo
|
||||||
- Bug fixes require regression tests
|
- Bug fixes require regression tests
|
||||||
- IMPORTANT: The `tests/client/test_client.py` is the most well designed test file. Follow its patterns.
|
- 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: 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
|
- Coverage: CI requires 100% (`fail_under = 100`, `branch = true`).
|
||||||
`uv run --frozen pytest -x` (coverage is configured in `pyproject.toml` with `fail_under = 100`
|
- Full check: `./scripts/test` (~20s, matches CI exactly)
|
||||||
and `branch = true`). If any branch is uncovered, add a test for it before pushing.
|
- 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:
|
- 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
|
- 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()`
|
- For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()`
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@
|
||||||
|
|
||||||
set -ex
|
set -ex
|
||||||
|
|
||||||
|
uv run --frozen coverage erase
|
||||||
uv run --frozen coverage run -m pytest -n auto $@
|
uv run --frozen coverage run -m pytest -n auto $@
|
||||||
uv run --frozen coverage combine
|
uv run --frozen coverage combine
|
||||||
uv run --frozen coverage report
|
uv run --frozen coverage report
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,6 @@
|
||||||
# pyright: reportUnknownArgumentType=false
|
# pyright: reportUnknownArgumentType=false
|
||||||
# pyright: reportUnknownMemberType=false
|
# pyright: reportUnknownMemberType=false
|
||||||
|
|
||||||
import sys
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
@ -65,12 +64,17 @@ async def test_direct_call_tool_result_return():
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_desktop(monkeypatch: pytest.MonkeyPatch):
|
async def test_desktop(tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
|
||||||
"""Test the desktop server"""
|
"""Test the desktop server"""
|
||||||
# Mock desktop directory listing
|
# Build a real Desktop directory under tmp_path rather than patching
|
||||||
mock_files = [Path("/fake/path/file1.txt"), Path("/fake/path/file2.txt")]
|
# Path.iterdir — a class-level patch breaks jsonschema_specifications'
|
||||||
monkeypatch.setattr(Path, "iterdir", lambda self: mock_files) # type: ignore[reportUnknownArgumentType]
|
# import-time schema discovery when this test happens to be the first
|
||||||
monkeypatch.setattr(Path, "home", lambda: Path("/fake/home"))
|
# 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
|
from examples.mcpserver.desktop import mcp
|
||||||
|
|
||||||
|
|
@ -85,15 +89,8 @@ async def test_desktop(monkeypatch: pytest.MonkeyPatch):
|
||||||
content = result.contents[0]
|
content = result.contents[0]
|
||||||
assert isinstance(content, TextResourceContents)
|
assert isinstance(content, TextResourceContents)
|
||||||
assert isinstance(content.text, str)
|
assert isinstance(content.text, str)
|
||||||
if sys.platform == "win32": # pragma: no cover
|
assert "file1.txt" in content.text
|
||||||
file_1 = "/fake/path/file1.txt".replace("/", "\\\\") # might be a bug
|
assert "file2.txt" in content.text
|
||||||
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
|
|
||||||
|
|
||||||
|
|
||||||
# TODO(v2): Change back to README.md when v2 is released
|
# TODO(v2): Change back to README.md when v2 is released
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue