test: per-tool wire contract, real-lifespan assembly test and a 98% coverage gate - #87
Merged
Merged
Conversation
.internal/ holds the ticket tracker and working spec for the test-confidence initiative. They are kept out of history until the tracked issues are filed and fixed, at which point the spec moves to docs/specs/.
tool_client/readonly_client move from test_tools.py:22-59 into tests/conftest.py so the new contract and assembly tests can use them. The move fixes three items from #80 on the way: - _make_mcp was annotated -> FastMCP but returned a 2-tuple; it is now a context manager and the fixtures are two lines each. - mcp._lifespan was restored after the with-block, so a failure inside Client(mcp) left the mock lifespan in place for every later test. The restore now sits in a finally. - test_client.py:16-17 built a GitLabClient per test and never closed it, leaking an httpx.AsyncClient each time. TestRequest now uses a conftest client fixture that closes on teardown, and the respx router comes from mock_api; the 14 @pytest.mark.asyncio markers were redundant under asyncio_mode = "auto" and are gone. The old config/client/mock_api fixtures in conftest.py:16-28 had no callers (coverage showed their bodies unexecuted); client and mock_api are reintroduced with real users, config is folded into client.
… tool Before this the suite asserted an outgoing request exactly once (test_tools.py, the page param on list_branches) and never executed the except arm of 75 of the 83 tools (servers/gitlab.py was 84% covered, the missing 168 lines being the `except Exception as e: return _err(e)` pairs). A change that mangled a POST body or dropped a kwarg passed green. test_tool_contract.py holds one row per @mcp.tool, derived by reading each tool body and the GitLabClient method it calls: (name, args, method, path, query, json body). test_request_shape mocks the route and checks all four against route.calls.last.request; test_forced_failure answers 404 on the same route and checks the _err key set, which is what runs the except arm. test_every_tool_has_a_row keeps the table equal to list_tools(). test_server_assembly.py boots through the real lifespan (gitlab.py:31-41, previously uncovered) and compares list_tools/resources/prompts against the decorator counts read from src/ at test time. test_exceptions.py gains the _err table: exact key set per exception type, so dropping body or hint from an envelope fails a test rather than a user. tests: 228 -> 405. src/ coverage: 88% -> 99%; servers/gitlab.py 84% -> 99%.
The coverage report mixed tests/ into the total (tests/test_links.py sat at 80% because its network cases are deselected), so the number was not the number of the package. source = ["src"] in pyproject.toml scopes it. Measured src/ after the contract tests: 98.76% (1537 statements, 19 missed). The gate is the floor of that. Proven locally: deselecting test_forced_failure drops src/ to 89.13% and pytest exits non-zero with "FAIL Required test coverage of 98% not reached".
vish288
force-pushed
the
harness-gitlab
branch
from
September 24, 2026 02:47
6b4fab3 to
7e4bedc
Compare
vish288
commented
Sep 25, 2026
vish288
left a comment
vish288
left a comment
Owner
Author
There was a problem hiding this comment.
Review: would approve — self-approval blocked by GitHub
Caught before merge: the branch was cut before the 0.10.2 release, so its pyproject/CHANGELOG/server.json/uv.lock diff reverted that bump (0.10.2 → 0.10.1). Rebased onto current main; the PR now touches only tests/, tests.yml, .gitignore and the coverage config. The worker had noticed the version mismatch but read the cause backwards.
Correctness. All 83 rows verified against list_tools() at test time (test_every_tool_has_a_row), so the table cannot drift from the registry. test_request_shape asserts method, raw path, query and decoded JSON body — the assertion this suite had exactly once before. test_forced_failure executes the except arm on every tool: servers/gitlab.py 84% → 99%.
Gate. --cov-fail-under=98 with source=["src"] so tests/ no longer inflates the number. Proven: deselecting test_forced_failure → 89%, build fails. Margin ~12 lines — tight but honest.
Verification. 405 passed, ruff and format clean, build ok, CI green on 3.10–3.13 after rebase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes the suite prove tool behaviour through the real MCP server, then locks that in CI. No changes under
src/.tests/unit/test_tool_contract.py: one row per@mcp.tool(83), derived by reading each tool body and theGitLabClientmethod it calls.test_request_shapeasserts method, path, query and JSON body ofroute.calls.last.request;test_forced_failureanswers 404 on the same route and asserts the_errkey set, which is what finally executes every tool'sexceptarm.test_every_tool_has_a_rowkeeps the table equal tolist_tools().tests/unit/test_server_assembly.py: boots through the real lifespan (servers/gitlab.py:31-41, previously uncovered) and compares tool/resource/prompt counts against the decorator counts read fromsrc/at test time; reads every resource.tests/unit/test_exceptions.py:_errtable, exact key set per exception type.tests/conftest.py:tool_client/readonly_clientlifted fromtest_tools.pyso all tool-level tests share them; lifespan restore moved intofinally;clientfixture that closes itshttpx.AsyncClient.pyproject.toml:[tool.coverage.run] source = ["src"]so the number is the package's number, nottests/mixed in..github/workflows/tests.yml:--cov-fail-under=98..gitignore:.internal/.Why
Before this, exactly one test asserted an outgoing request and 75 of 83 tools never had their
exceptarm run. A refactor that mangled a POST body or dropped a kwarg passed green. The error-contract change (#75, #76) is only safe once this exists.Numbers
src/coverageservers/gitlab.pyGate proof, run locally with
--cov-fail-under=98: deselectingtest_forced_failuredropssrc/to 89.13% and pytest exits non-zero withFAIL Required test coverage of 98% not reached. Deselecting a single row leaves 99%; the margin at 98 is about 12 lines.Housekeeping absorbed from #80
The
tests/items only: redundant@pytest.mark.asynciointest_client.py,_make_mcpannotated-> FastMCPwhile returning a tuple,mcp._lifespanrestore that never ran ifClient(mcp)raised, unclosedGitLabClientintest_client.pyandconftest.py. Thesrc/items in #80 are untouched here.Also found on the way: the
config/client/mock_apifixtures inconftest.pyhad no callers (coverage showed their bodies unexecuted).clientandmock_apinow backtest_client.py;configis gone.