test(e2e): add coverage registry and collector - #32304
Conversation
Introduce the e2e coverage denominator: 282 behavior cells across the six tracking modules (LLMs, MCPs, Management/UI, Reliability & Performance, Logging & Guardrails, Other), one validated YAML row each, plus a collector that diffs the registry against @pytest.mark.covers markers and reports coverage per module. The registry rows validate against a pydantic discriminated union so a row cannot carry a field from another module. The collector is static: a collect-only pass reads the markers, so it runs no test and needs no live proxy. Register the covers marker suite-wide so that pass works under --strict-markers. This is a draft for review. Tiers are proposed rather than signed off, and a few cells still need a support check or a prune.
…itellm_/kind-wright-84af2b
Greptile SummaryAdds
Confidence Score: 4/5Safe to merge — all changes are confined to the test tooling directory with no production code affected; the only shared file touched is The new coverage tooling is well-structured and the tests pass. The three findings are all in the tooling itself: stderr leakage from the pytest subprocess can make CLI output noisy, the
|
| Filename | Overview |
|---|---|
| tests/e2e/coverage_registry/collector.py | New CLI tool that runs a pytest collect-only pass and diffs the registry against @pytest.mark.covers markers; stdout is suppressed but stderr is not, so collection warnings can pollute terminal output. |
| tests/e2e/coverage_registry/schema.py | Pydantic discriminated union for registry cells; ROLLUP dict coupling with module literals is not exhaustiveness-verified and could KeyError if a new Cell subtype is added without updating ROLLUP. |
| tests/e2e/coverage_registry/registry.py | Loads and validates YAML cells via Pydantic; correctly rejects duplicate ids; does not validate that each cell's id prefix matches its module field. |
| tests/e2e/coverage_registry/test_collector.py | Unit tests for coverage math and registry loader; includes a canary test asserting > 250 cells and unique ids. All test logic is correct. |
| tests/e2e/conftest.py | Registers the covers marker suite-wide so collect-only passes work under --strict-markers; minimal change, no issues. |
Comments Outside Diff (1)
-
tests/e2e/coverage_registry/registry.py, line 678-686 (link)Cell
idprefix not validated againstmoduleThe schema enforces the
modulefield via aLiteraldiscriminator, but theidfield is a plainstrwith no check that its first segment matchesmodule. A cell withmodule: llmandid: mcp.somethingpasses schema validation and duplicate detection, but_module_coverageclassifies it byc.module("llm"), so the coverage numbers for both the LLMs and MCPs modules would be silently wrong. Adding amodel_validatorto_Base(or a post-load check here) that assertsid.startswith(c.module + ".")would catch this class of typo.
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile
| with contextlib.redirect_stdout(io.StringIO()): | ||
| pytest.main( |
There was a problem hiding this comment.
The
collect_covered_ids function redirects stdout into a StringIO buffer to suppress pytest's collection output, but stderr is left open. When collection encounters import errors, missing-fixture warnings, or deprecation notices, those messages are written to stderr and appear in the terminal interleaved with the coverage table, making the output noisy and harder to read. Redirecting stderr in the same with block suppresses the noise while still capturing errors via sink.collection_errors.
| with contextlib.redirect_stdout(io.StringIO()): | |
| pytest.main( | |
| with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): | |
| pytest.main( |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| def _module_coverage(module: str, cells: tuple[Cell, ...], covered: frozenset[str]) -> ModuleCoverage: | ||
| in_module = tuple(c for c in cells if ROLLUP[c.module] == module) |
There was a problem hiding this comment.
_module_coverage calls ROLLUP[c.module] for every cell. ROLLUP is defined in schema.py alongside the Cell union, but there is no exhaustiveness check ensuring the two stay in sync. If a new module literal is added to Cell (e.g., Literal["agents"]) and ROLLUP is not updated, this line raises KeyError at runtime for every cell of that new type, crashing the collector entirely. A get with a fallback or an explicit assertion at import time would catch the mismatch earlier.
| def _module_coverage(module: str, cells: tuple[Cell, ...], covered: frozenset[str]) -> ModuleCoverage: | |
| in_module = tuple(c for c in cells if ROLLUP[c.module] == module) | |
| def _module_coverage(module: str, cells: tuple[Cell, ...], covered: frozenset[str]) -> ModuleCoverage: | |
| in_module = tuple(c for c in cells if ROLLUP.get(c.module, "") == module) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
mubashir1osmani
left a comment
There was a problem hiding this comment.
merging this, we will start adding coverage for all p0 tiers and keep moving forward from there
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5ecc6e1. Configure here.
Relevant issues
Part of the ongoing effort to quantify and track e2e test coverage per component and drive escaped regressions down. This PR lands the first concrete piece: the denominator and the tool that measures against it
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
This is test tooling rather than a proxy behavior change, so there is no endpoint to curl; the demonstration is the collector's own output. Running it against the current suite reports the real baseline coverage and flags markers that do not line up with the registry
The tooling tests pass as well:
Type
✅ Test
Changes
This adds
tests/e2e/coverage_registry/, the set of e2e behaviors we want covered, one validated row per behavior. A cell is one customer-noticeable behavior a single test can assert pass/fail on, for examplellm.chat_completions.bedrock_converse.tool_use.stream.works. There are 282 cells across the six tracking modules (125 of them P0), enumerated from the codebase and groupedmodule > feature > testper the grammar already documented intests/e2e/CLAUDE.mdThe rows live in per-prefix YAML files and validate against a pydantic discriminated union in
schema.py, so a row cannot carry a field from another module and duplicate ids are rejected at load time.collector.pydiffs the registry against the@pytest.mark.coversmarkers on the tests and reports coverage per module. It is static: a collect-only pass reads the markers, so it runs no test and needs no live proxy. It also lists markers that point at ids not in the registry, so a typo or an unenumerated behavior surfaces instead of being silently dropped. Thecoversmarker is now registered suite-wide in the shared conftest so that collect-only pass works under--strict-markersPlease read this as a draft for review rather than a finished set. The cells were enumerated from the code and the tiers are a first proposal; the things worth settling before treating the denominator as final are written up in
tests/e2e/coverage_registry/README.md(tier sign-off, a few cells that need a support check or prune, the auth double-coverage boundary, and the deliberately P0-weighted smoke cohorts)Note
Low Risk
Test-only infrastructure under
tests/e2e/; no proxy or runtime behavior changes.Overview
Introduces a checked-in e2e coverage denominator under
tests/e2e/coverage_registry/: ~282 validated cells (125 P0) across six dashboard modules, stored in per-prefix YAML and enforced by a Pydantic discriminated union inschema.py(duplicate ids fail at load).Tests can declare coverage with
@pytest.mark.covers("cell.id"), registered intests/e2e/conftest.pyfor--strict-markers.collector.pyruns a static pytest collect-only pass, diffs markers against the registry, and prints per-module totals with P0 as the headline; it also surfaces orphan markers and collection import failures.registry.pyloads YAML;test_collector.pycovers the math and a real-registry canary. README notes the cell set and tiers are still a draft for review.Reviewed by Cursor Bugbot for commit 5ecc6e1. Bugbot is set up for automated code reviews on this repo. Configure here.