--- title: "test-quality by modelcontextprotocol · skilld" canonical_url: "https://skilld.dev/gh/modelcontextprotocol/python-sdk" meta: description: "Test quality bar for this repo. Read when writing, reviewing, or designing tests — covers naming, abstraction level, assertions, determinism, and the process… From modelcontextprotocol/python-sdk." "og:description": "Test quality bar for this repo. Read when writing, reviewing, or designing tests — covers naming, abstraction level, assertions, determinism, and the process… From modelcontextprotocol/python-sdk." "og:title": "test-quality by modelcontextprotocol" "twitter:description": "Test quality bar for this repo. Read when writing, reviewing, or designing tests — covers naming, abstraction level, assertions, determinism, and the process… From modelcontextprotocol/python-sdk." "twitter:title": "test-quality by modelcontextprotocol" --- ` [All skills](https://skilld.dev/skills) [![modelcontextprotocol avatar](https://skilld.dev/_img/avatar?url=https%3A%2F%2Fgithub.com%2Fmodelcontextprotocol.png%3Fsize%3D96)](https://skilld.dev/gh/modelcontextprotocol) # **/test-quality** [@1b92efe](https://github.com/modelcontextprotocol/python-sdk/commit/1b92efedc83303f4fecb2a18569e1a05b137cab7 "Your agent reads SKILL.md at commit 1b92efe") by [Model Context Protocol](https://skilld.dev/gh/modelcontextprotocol)· [modelcontextprotocol](https://skilld.dev/gh/modelcontextprotocol)/ [python-sdk](https://skilld.dev/gh/modelcontextprotocol/python-sdk)·24k stars 3,995 Test quality bar for this repo. Read when writing, reviewing, or designing tests — covers naming, abstraction level, assertions, determinism, and the process for agent-driven test work. - 1 file - 10.1 KB - Updated 3 months ago - [GitHub](https://github.com/modelcontextprotocol/python-sdk/blob/1b92efedc83303f4fecb2a18569e1a05b137cab7/.claude/skills/test-quality/SKILL.md "View SKILL.md on GitHub") - [No alerts](#third-party-checks "Third-party checks: No alerts · 3 checks · Risk SAFE") ## SKILL.md 10.1 KB **≈50** tokens always: the name and description. **≈2.5k** when used: this file. ## Test & code quality guide What "best practice for new work" means in this repo. Each rule carries its recorded reasoning where one exists; a rule with no stated why is a convention — follow it anyway. ### Naming & shape - **Test names are behaviour sentences** stating the observable outcome, not the feature being poked: `test_elicit_form_decline_returns_no_content`, never `test_elicit_form_decline`. - Plain top-level `test_*` functions; no `Test` classes (legacy files have them — don't copy). - **Docstrings: 1–2 sentences of behaviour, honest about provenance** — spec-mandated, SDK-defined, or pinning a known gap? Say which: provenance is the triage key when the test later fails. A pinned-gap assertion breaking usually means a change *fixed* the gap; a spec-mandated assertion breaking means a regression. - Define things in dependency order; nothing forward-references. For client↔server tests: handlers → server construction → client setup → act → assert — the test reads in the order the conversation happens. - Inline the server (or equivalent setup) in the test, so the whole observable behaviour fits on one screen. Lift to a file-level fixture only when several tests in *that file* genuinely share it; never share across files. - A big multi-step test is fine when the property is irreducibly multi-step (e.g. resumability). Split when a failure wouldn't tell you which claim broke — not for shortness. Compensate with a numbered "Steps:" docstring so a reader sees the choreography before the body. ### Level of abstraction - **Drive through the highest-level public API that can observe the property.** Hand-built wire requests are brittle and don't prove the user-facing contract; tests that stay above the internals keep working when the internals change. Drop to raw HTTP only when the assertion is about something the high-level API *cannot* observe (status codes, headers, wire framing). - **Scripting a peer over raw streams is a last resort**, reserved for behaviour the typed API cannot *produce* (malformed input, an impossible peer response). First ask what it would take for the public API to express it — often a small helper suffices. Every such test's docstring states why the public API couldn't do it. - **In-memory / in-process first.** HTTP-, SSE-, and auth-shaped behaviour can all be driven through an in-process ASGI transport; threads only when necessary, subprocesses only when the process boundary is itself the thing under test. In-process isn't just faster — it surfaces bugs (a real stream leak was found this way) that subprocess indirection masks. - **Tests read like real user code**: no aliasing shims in conftest, no walls of suppressions, no private imports unless that is genuinely the documented way to do the thing. - The assertion must prove the round trip — no side-channel state. What the server saw comes back through the protocol, or via a closure-captured list asserted after the call. Handlers assert their dispatch identity first ( `assert params.name == "add"`), proving the request that arrived is the request the test sent. ### Assertions - **Transformations** (input → output the SDK produced) → full-object `snapshot(...)` equality, so an added or dropped field fails. Regenerate with `--inline-snapshot=fix` so intentional changes arrive as a reviewable diff; never hand-edit snapshot literals. - **Pass-through values** (opaque tokens, `_meta`, cursors) → identity against the same variable you sent. A snapshot of a pass-through value only "matches" because a human checked two literals correspond — it proves nothing. - **Errors** → `pytest.raises` + `.code` against the named constant; snapshot SDK-authored messages; never `match=` on message text. Third-party text (pydantic, jsonschema) → stable prefix only — never pin text that changes with a dependency upgrade — with a comment saying so. ### Determinism - **No sleeps, ever.** A sleep guesses at timing instead of waiting on the condition, so it either flakes or pads the run — sleeps head the list of the older test code's failure modes. Coordinate with `anyio.Event` so the wait ends exactly when the condition holds (that discipline is why 529 e2e tests run in \~10 s). Sole exception: tests *of* time-based features, with a comment. - Bound every indefinite wait with `anyio.fail_after(5)`. **5 is the standard** — widening it needs an articulable reason; "10 to be safe" is covering up a flake, not fixing one, and unexplained widenings propagate (one agent used 10 and every later one copied it). - **Never assert wall-clock time, even with huge margins.** `elapsed < 0.9` on a \~0.01 s operation — a 90× margin — was still rejected in review. `fail_after` bounding a hang is the only timing primitive allowed. - **Concurrency tests must prove genuine interleaving.** Without barriers the scheduler is free to serialize — "a" can start, finish, and return before "b" even begins — so two `start_soon` calls prove nothing. Gate with events so all parties are mid-flight before any proceeds, emit interleaved, then assert the demux. - **Don't over-synchronize either.** When delivery ordering is guaranteed (notifications emitted during a request you're awaiting, over a single ordered in-memory stream), a plain collected list asserted after the call is correct; events are for messages not tied to an awaited operation. Verify the ordering guarantee actually holds for your transport first. - Async tests use anyio, not asyncio. ### Behaviour philosophy - **Pin current behaviour; never xfail.** A green suite asserting what actually happens is a regression bar for any refactor; an xfail proves nothing about it. Where current behaviour falls short of spec, pin the divergent output and record the gap as data — a tracking issue, with the docstring naming the known gap (suites with a requirements manifest record it as a divergence entry). Not hidden, not skipped. - **Hollow-proof check**: before claiming a test covers a behaviour, re-read the claim and ask "which assertion proves *this*?" A passing test near a behaviour is not proof of it — a full review of the e2e suite found two such cases even under this discipline. ### Hygiene - No new `# pragma`, `# type: ignore`, `# noqa` by default — restructure first; a suppression usually means a test or a type is missing. The narrow sanctioned escape hatches (and the audit to run before pushing) are in AGENTS.md. In tests, narrow types with `assert isinstance`; never `Any`/ `object` when a real type exists. - **Warnings raised during tests are findings, not noise** (the repo runs `filterwarnings = error`). Fix the cause; if the fix can't land in the same change, scope the suppression to the one fixture that needs it and track the real fix explicitly. - Registered-but-never-invoked handler bodies are `raise NotImplementedError`, so they cannot silently become load-bearing. - Comments live next to the line they explain, not in docstrings; single backticks for code refs; match the surrounding comment density (one-liners next to one-liners). No `from __future__ import annotations` (py310+ repo). - Test work doesn't change `src/` as a side effect — the one mechanical exception is deleting a pragma a new test now covers. If a test can't be written without a library change, raise it as a finding or defer the test; don't quietly edit `src/`. ### Process (for agent-driven work) - **Small chunks.** ≤10 tests per human-reviewed batch; when fanning out to multiple agents, ≤5 per agent. Review quality scales inversely with batch size: the observed shortcuts (over-stacked tests, a timing assertion, wrong abstraction level) all surfaced in one oversized 27-test batch. - **Design → review → implement, as separate steps.** The design deliverable is not just the plan — it's the judgement calls (abstraction level, deferrals, contested assertions) stated explicitly, so the reviewer vetoes them before implementation rather than discovering them in the diff. - **High-stakes areas get fresh adversarial reviewers** on both the design and the implementation — fresh, because whoever designed it (or saw the proposed fix) anchors on the same layer. SERIOUS findings (wrong assertion, missed MUST/SHOULD, an unrecorded known gap) re-loop; style doesn't. Reserve the full panel for areas where being wrong is worse than being slow. - **Investigations pair a full-context look with a fresh agent given only the problem** — never the proposed fix — and compare conclusions. The unbiased read regularly catches anchoring on the wrong layer. - **When review questions a decision, reconsider genuinely**: re-derive the tradeoff, state the options, recommend with reasons. Defending the original choice is a valid outcome; reflexively agreeing with every challenge is as bad as ignoring it. - **Validate the reference artifact before building on it.** Whatever you treat as ground truth (a spec import, a baseline, a generated list), check it *first* — discovering it was incomplete after five batches costs far more than before batch one. - **Verify as you go**: per-file `uv run --frozen pytest -q` + pyright + ruff while iterating; full suite, coverage, and `./scripts/test` (when `src/` was touched) at integration. - **Every agent writes notes**: what it did, what broke, and — most importantly — what it couldn't decide, so open questions surface instead of being silently resolved by whoever hit them. - **Quality over speed is a stated goal, not a preference** ("rushing something out the door is not the goal here, explicitly so"). Don't quietly de-scope agreed work mid-stream; renegotiate scope explicitly. - **Don't copy patterns from existing test code by default** — much of the repo's older test code is below the current bar (sleeps, mocks, `Test` classes, raciness). These rules define the bar for new work. Source: [SKILL.md on GitHub](https://github.com/modelcontextprotocol/python-sdk/blob/1b92efedc83303f4fecb2a18569e1a05b137cab7/.claude/skills/test-quality/SKILL.md) ## Third-party checks
No alerts1mo3 checks · Risk SAFE - Gen Agent Trust Hub1mo This skill is a documentation-only guide providing best practices for software testing and code quality. It contains no executable code, remote resource fetching, or malicious patterns. - Socket1mo No alerts - Snyk1mo Risk: LOW · No issues
## Provenance [Signed by skilld at 1b92efe.](https://github.com/modelcontextprotocol/python-sdk/commit/1b92efedc83303f4fecb2a18569e1a05b137cab7 "1b92efedc83303f4fecb2a18569e1a05b137cab7") This ties the file your Agent reads to that commit on GitHub. It does not review the instructions. Last checked against GitHub 16 hours ago. Activeupdated 3 months ago ## README badge ![README badge for modelcontextprotocol/python-sdk](https://skilld.dev/b/modelcontextprotocol/python-sdk?theme=light&label=0) ## Related skills - - - - - -