diff --git a/tests/e2e/CLAUDE.md b/tests/e2e/CLAUDE.md index 17aee22560c..6af157f1ad8 100644 --- a/tests/e2e/CLAUDE.md +++ b/tests/e2e/CLAUDE.md @@ -4,25 +4,22 @@ Code-style rules for writing tests under `tests/e2e/`. The harness already encod ## Suite folders -Each subdirectory under `tests/e2e/` is one suite, scoped to an endpoint family or behavior area. If you add a new folder, you must add a line here describing what kind of tests belong in it, so the layout stays self-describing. `gateway/` is the exception: it holds proxy configuration only and never tests +Each subdirectory under `tests/e2e/` is one suite, scoped to an endpoint family or behavior area. If you add a new folder, you must add a line here describing what kind of tests belong in it, so the layout stays self-describing. `coverage_registry/` is the exception: it holds the coverage denominator and its collector, not a suite -- `llm_translation/` - LLM endpoint and provider-translation behavior: passthrough, custom pricing, OCR, and the non-chat inference endpoints (`/v1/responses`, `/v1/messages`, `/embeddings`, `/v1/rerank`, `/v1/audio/speech`, `/v1/images/generations`), each against a deployment the test creates via `/model/new` and deletes on teardown +- `llm_translation/` - LLM endpoint and provider-translation behavior: passthrough, custom pricing, OCR, and the non-chat inference endpoints (`/v1/responses`, `/v1/messages`, `/embeddings`, `/v1/rerank`, `/v1/audio/speech`, `/v1/images/generations`), each against a deployment the test creates via `/model/new` and deletes on teardown. Realtime websocket sessions, including the pipecat audio path, live in its `realtime/` subfolder - `access_control/` - the gateway's authorization and error-shape contract: per-key model allow-lists, route-group permissions (`allowed_routes`), and unknown-model validation -- `embeddings/` - the `/embeddings` endpoint across providers -- `batches/` - the `/batches` endpoint (placeholder until the first test lands) -- `realtime/` - realtime websocket sessions, including the pipecat audio path +- `batches/` - the `/batches` endpoint: the file-upload/create/retrieve/cancel lifecycle per provider +- `guardrails/` - guardrail providers at each hook point: what they block, mask, or allow, and how team-level config turns a global guardrail off - `quota_management/` - quota enforcement and accounting, one subfolder per behavior: `ratelimit/` (rpm/tpm blocks, window reset, pacing headers on live traffic), `budgets/` (budget definition, enforcement, and reset windows: key, team, tag, soft, multi-window), and `spend_tracking/` (spend logging and cost attribution on `/spend/*`) - `management/` - key/team/user/organization management routes: create/update/delete persistence via the info routes, team membership, and llm-only-key route denials (API surface; not Playwright) - `a2a/` - the A2A (agent-to-agent) surface: admin registration via `/v1/agents`, proxy-fronted card discovery at `/.well-known/agent-card.json`, and JSON-RPC `message/send` invocation, driving agents backed by the litellm completion bridge (a real provider) and asserting protocol-version normalization (0.3 vs 1.0) - `mcp/` - the MCP server surface over api_key auth against the real Datadog remote MCP server (see "MCP suite: real Datadog only" below); plus the gateway-managed OAuth (authorization_code) path exercised through `/chat/completions`, the one behavior Datadog's static-header auth cannot reach, seeding the per-user upstream token via the interactive authorize dance driven with the mcp SDK's own OAuth client (headless-browser consent from a saved session) and asserting the completion lists and executes the server's tools with the stored per-user token - `logging/` - logging-integration delivery (datadog and friends) -- `security/` - secret handling and log-leak protection - `router/` - routing and reliability behavior (fallbacks, cooldowns) - `load/` - throughput/performance under concurrency: drives real concurrent traffic through the whole stack with Locust and asserts a throughput SLO; marked `load` so the parent conftest collects it last and it never perturbs latency-sensitive suites. Also home of the weekly session-anomaly test (`test_weekly_session_anomaly_e2e.py`): Claude Code-shaped multi-turn sessions against real providers with ceilings on error rate, cache read/write, turn time, and spend; additionally marked `weekly` and deselected unless `E2E_WEEKLY_ANOMALY` is set, because it spends real provider money (driven by `.github/workflows/weekly_load_anomaly.yml`) - `other/` - the holding-pen suite for the `other.*` registry cluster with no home of its own yet: the master-key auth gate and the process-lifecycle health probes (liveness, public readiness, authenticated readiness diagnostics). Promote a cluster out once it is large/stable enough for its own suite -- `gateway/` - proxy configuration only (`litellm-config.yml`); no tests - `claude_code/` - the Claude Code compatibility matrix: drives the real `claude` CLI (and HTTP probes) against a proxy for each feature x provider cell, reporting tagged-union outcomes via the `compat_result` fixture; ships its own driver/builder/publisher plus `_*_unit_tests/` trees. The HTTP probes ride the shared transport (`ProxyClient.count_tokens` / `ProxyClient.messages`); the CLI-driving path stays bespoke -- `ui/` - the Admin UI browser suite: Playwright in TypeScript, driving the dashboard served by a live proxy on port 4000 (seeded postgres + mock LLM upstream; see its `run_e2e.sh`). It is a self-contained npm package with its own lockfile and does not use the Python harness, pytest markers, or the shared transport; the Python rules in this file (typed models, `Result` unions, basedpyright zero-error gate) do not apply inside it. Its only Python file, `fixtures/mock_llm_server/server.py`, is excluded from the e2e basedpyright gate via the root `pyrightconfig.json` +- `ui/` - the Admin UI browser suite: Playwright in TypeScript, driving the dashboard served by a live proxy on port 4000 (seeded postgres + mock LLM upstream; see its `run_e2e.sh`). It is a self-contained npm package with its own lockfile and does not use the Python harness, pytest markers, or the shared transport; the Python rules in this file (typed models, `Result` unions, basedpyright zero-error gate) do not apply inside it. Its only Python file, `fixtures/mock_llm_server/server.py`, is excluded from the e2e basedpyright gate via the root `pyrightconfig.json`. Emitting no pytest markers, it declares the coverage cells it proves in `coverage.yaml` instead (see "Coverage registry" below) ## MCP suite: real Datadog only @@ -79,6 +76,8 @@ The harness is fully typed with no error budget: `make lint-e2e-basedpyright` mu The set of tests we want is a registry checked into this repo, one row per behavior; that file is the definition of done and the denominator. Each e2e test declares what it covers with `@pytest.mark.covers("...")`, and a small collector diffs the registry against the tests and ships coverage to the existing Grafana. No Allure, no new dependencies +The number is a property of the source tree, not of the run: a cell counts as covered when a test declaring it exists, whether or not that test is selected, skipped, or passing on this machine. The collector reads the markers with `ast` so a deselected test (the `weekly` load test) and a module behind `pytest.importorskip` both keep counting; it runs a collect-only pytest pass only for markers built at import time and for nodes that fail to import. The TypeScript `ui/` suite emits no markers, so it declares its cells in `tests/e2e/ui/coverage.yaml`; add a row there when a UI spec proves a registry cell, and a wrong id shows up as an orphan marker just like a mistyped `covers`. Each row names the spec and the Playwright test title behind it and both are resolved against the tree, so renaming or deleting that test drops the cell and fails `--strict` rather than leaving it counted forever + Coverage is organized as module > feature > test. Dashboard modules are `Core LLMs`, `Non-Core LLMs`, `MCPs`, `Management/UI`, `Reliability & Performance`, `Quota Management`, `Logging & Guardrails`, and `Other`. The Loki stdout formatter maps those display modules to log-safe labels (`core_llms`, `non_core_llms`, `mcp`, `management_ui`, `reliability_performance`, `quota_management`, `logging_guardrails`, and `other`) without changing JSON or Prometheus labels. A feature is either an endpoint (`/chat/completions`) or a behavior (fallbacks, rate limits; config-driven, with no route of its own). A cell reads like `llm.chat_completions.bedrock_converse.tool_use.stream.works` The metric is coverage: the share of registry rows that have a passing covering test, reported to Grafana per module so a gap surfaces as an uncovered row rather than a silent absence diff --git a/tests/e2e/coverage_registry/README.md b/tests/e2e/coverage_registry/README.md index aef4c16c89a..4e0e225afed 100644 --- a/tests/e2e/coverage_registry/README.md +++ b/tests/e2e/coverage_registry/README.md @@ -34,8 +34,34 @@ def test_openai_streaming_tool_calls(self) -> None: ## The number `collector.py` diffs the registry against those markers 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. Whether a covered cell currently passes or fails is a separate, live concern. +It is static in the strong sense: a cell is covered when a test declaring it exists in +the tree, so the number is a property of the source and does not move with the runner. +The markers are read off the source with `ast`, which keeps deselected tests (the +`weekly` load test, gated on `E2E_WEEKLY_ANOMALY`) and modules behind an optional +dependency (`pytest.importorskip("mcp")`) counted on every machine. A collect-only +pytest pass runs alongside it for the two things the source text cannot give: markers +built at import time (`pytest.mark.covers(*fn(...))` inside a `pytest.param`) and the +nodeids that failed to import, whose cells are genuinely unknowable. It runs no test +and needs no live proxy. Whether a covered cell currently passes or fails is a +separate, live concern. + +The Playwright suite under `tests/e2e/ui/` is TypeScript and emits no pytest markers, +so it declares the cells it covers in `tests/e2e/ui/coverage.yaml` and the collector +unions those ids in. An id there that is not in the registry surfaces as an orphan +marker exactly like a typo in a pytest marker. + +Each row names the `spec` it lives in and the Playwright `test` title that proves it, +and the collector resolves both against the tree on every run. Rename or delete that +test and the row stops counting and is listed as a stale declaration, so a claim +cannot outlive the test behind it. Comments are stripped before titles are read, so +a test commented out and forgotten fails the same way a deleted one does, and the +strip is string-aware so a `//` inside a title is never mistaken for a comment +opener. An interpolated title is matched against its +literal segments, so it is still checked as far as it can be, and the check is per +row rather than per file: a dynamic title elsewhere in the spec never exempts a row +that names a literal one. A title with no literal text, or one assembled from +variables, backs no row at all, so the contributor has to give that test a +matchable title instead of the check being waved through. ``` cd tests/e2e && PYTHONPATH=. python -m coverage_registry.collector @@ -64,8 +90,9 @@ cd tests/e2e && PYTHONPATH=. python -m coverage_registry.collector --strict ``` Strict mode exits non-zero on `@pytest.mark.covers(...)` ids that are not checked into -the registry. Add `--fail-on-collection-errors` when the job should also fail on pytest -collection errors. +the registry, and on UI declarations whose spec or test title no longer exists. Both are +the same failure: the registry claims something the tree does not support. Add +`--fail-on-collection-errors` when the job should also fail on pytest collection errors. ## Status: this is a draft for review @@ -74,8 +101,13 @@ things to settle before treating the set as final: - tiers are proposed, not signed off; 125 P0 is a lot to prove fail-before-fix, so P0 may want tightening -- a few cells need a support check or a prune (for example `llm.embeddings.anthropic.*` - and `reliability.perf.throughput.under_slo`) +- `llm.embeddings.anthropic.basic.nonstream.works` was removed: Anthropic ships no + embeddings API, litellm has no anthropic embedding handler (the one on the chat + handler is a `pass` stub and no anthropic row in + `model_prices_and_context_window.json` carries `mode: embedding`), so the row could + never pass. `reliability.perf.throughput.under_slo` still needs a support check +- prune a row only when it is genuinely unreachable, with the evidence written down + here; the denominator is the metric, and shrinking it for any other reason games it - auth is covered in two places (`other.auth.*` and the mgmt authz assertions); the boundary needs a decision, and the auth cluster may deserve promotion to its own module - the P2 "niche" cells each stand in for a large tail of integrations/providers by design, diff --git a/tests/e2e/coverage_registry/collector.py b/tests/e2e/coverage_registry/collector.py index 50ef23bcb1a..f0f238564dd 100644 --- a/tests/e2e/coverage_registry/collector.py +++ b/tests/e2e/coverage_registry/collector.py @@ -1,18 +1,31 @@ """Diff the registry (denominator) against the @pytest.mark.covers markers on the live tests (numerator) and report coverage per module. -Coverage here is static: it reads the markers via a collect-only pass, so it runs -no test and needs no live proxy. Whether a covered cell currently passes or fails -(covered_pass vs covered_fail) is a separate, live concern layered on top later. +Coverage here is static: a cell is covered when a test declaring it exists in the +source tree. Whether that test is deselected on this run, skipped for a missing +optional dependency, or currently passing is a runtime concern and must not move +the number, so the markers are read straight off the source with `ast` rather +than off whatever pytest happened to keep after collection. A collect-only pytest +pass still runs alongside it, for two things the source text cannot give: markers +built at import time (`pytest.mark.covers(*fn(...))` inside `pytest.param`) and +the nodeids that failed to import, whose cells are genuinely unknowable. + +The TypeScript Playwright suite under `tests/e2e/ui/` emits no pytest markers, so +it declares the cells it covers in `tests/e2e/ui/coverage.yaml` instead. Each row +names the spec and the test title that prove it, and both are resolved against +the tree, so deleting or renaming that Playwright test drops the cell out of the +numerator and fails `--strict` the same way an unknown cell id does. cd tests/e2e && PYTHONPATH=. python -m coverage_registry.collector """ from __future__ import annotations +import ast import contextlib import io import json +import re import sys from argparse import ArgumentParser from dataclasses import dataclass @@ -20,12 +33,204 @@ from pathlib import Path from typing import Literal import pytest -from pydantic import BaseModel +import yaml +from pydantic import BaseModel, ConfigDict from .registry import load_registry from .schema import MODULE_ORDER, Cell, Tier, dashboard_module, loki_module_label E2E_DIR = Path(__file__).resolve().parent.parent +UI_DECLARATION_FILE = E2E_DIR / "ui" / "coverage.yaml" + +_UNSCANNED_DIRS: frozenset[str] = frozenset( + {"__pycache__", ".git", ".venv", "node_modules", "site-packages", "venv"} +) + + +def _is_covers_call(func: ast.expr) -> bool: + """True for `.mark.covers` and the `from pytest import mark` spelling.""" + if not isinstance(func, ast.Attribute) or func.attr != "covers": + return False + owner = func.value + if isinstance(owner, ast.Attribute): + return owner.attr == "mark" + return isinstance(owner, ast.Name) and owner.id == "mark" + + +def _covers_ids_in_source(source: str) -> frozenset[str]: + tree = ast.parse(source) + return frozenset( + arg.value + for node in ast.walk(tree) + if isinstance(node, ast.Call) and _is_covers_call(node.func) + for arg in node.args + if isinstance(arg, ast.Constant) and isinstance(arg.value, str) + ) + + +def _scannable(path: Path) -> bool: + return not _UNSCANNED_DIRS.intersection(path.parts) + + +def _read_covers_ids(path: Path) -> frozenset[str] | None: + """The file's declared cell ids, or None when it cannot be parsed.""" + try: + return _covers_ids_in_source(path.read_text(encoding="utf-8")) + except (OSError, SyntaxError, UnicodeDecodeError): + return None + + +def scan_covers_markers( + e2e_dir: Path = E2E_DIR, +) -> tuple[frozenset[str], tuple[str, ...]]: + """Return (cell ids declared by a `covers` marker anywhere in the source tree, + paths that could not be parsed). Independent of deselection, of env-gated + opt-ins, and of optional dependencies, because nothing is imported.""" + parsed = tuple( + (path, _read_covers_ids(path)) + for path in sorted(e2e_dir.rglob("*.py")) + if _scannable(path) + ) + return ( + frozenset( + cell_id for _, ids in parsed if ids is not None for cell_id in ids + ), + tuple(str(path) for path, ids in parsed if ids is None), + ) + + +class _UiCoveredCell(BaseModel): + model_config = ConfigDict(frozen=True, extra="forbid") + + id: str + spec: str + test: str + + +class _UiDeclaration(BaseModel): + model_config = ConfigDict(frozen=True, extra="forbid") + + covers: tuple[_UiCoveredCell, ...] = () + + +@dataclass(frozen=True, slots=True) +class UiDeclarations: + """Cells the Playwright suite claims, split by whether the claim still holds.""" + + ids: frozenset[str] + unresolved: tuple[str, ...] + + +_TITLED_CALL = re.compile( + r"\btest(?:\.(?:describe|only|skip|fixme|serial|parallel|step))*\s*\(\s*" +) +_TITLE_LITERAL = re.compile(r"(['\"`])((?:\\.|(?!\1).)*?)\1\s*[,)]", re.DOTALL) +_INTERPOLATION = re.compile(r"\$\{[^{}]*\}") + +_STRING_OR_COMMENT = re.compile( + r""" + (?P '(?:\\.|[^'\\\n])*' + | "(?:\\.|[^"\\\n])*" + | `(?:\\.|[^`\\])*` ) + | (?P //[^\n]* | /\*.*?\*/ ) + """, + re.VERBOSE | re.DOTALL, +) + + +def _without_comments(source: str) -> str: + """The source with its comments blanked out, so a commented-out test cannot + back a declaration. String literals are matched by the same pass and kept + intact, so a `//` inside a title (a URL, say) is never mistaken for a comment + opener; a false negative there would be worse than the staleness this catches.""" + return _STRING_OR_COMMENT.sub( + lambda match: match.group("string") or " ", source + ) + + +@dataclass(frozen=True, slots=True) +class _SpecTitles: + """What a spec's test titles can be matched against: the ones written out in + full, and a pattern per interpolated title covering the titles it can produce.""" + + literal: frozenset[str] + patterns: tuple[re.Pattern[str], ...] + + def covers(self, title: str) -> bool: + return title in self.literal or any( + pattern.fullmatch(title) for pattern in self.patterns + ) + + +def _title_pattern(template: str) -> re.Pattern[str] | None: + """A pattern for the titles an interpolated title can produce, or None when it + is all interpolation and would therefore match anything.""" + segments = tuple(_INTERPOLATION.split(template)) + if not any(segment.strip() for segment in segments): + return None + return re.compile(".*".join(re.escape(segment) for segment in segments), re.DOTALL) + + +def _spec_titles(spec_source: str) -> _SpecTitles: + """Every test title the spec can produce, read from the whole `test` family. + + A first argument that is not a string literal contributes nothing: that covers + `test.skip(condition, reason)`, which shares its name with the titled form, and + a title assembled from variables, which cannot be matched by text at all. A + declaration naming one of those fails loudly rather than being waved through.""" + source = _without_comments(spec_source) + titles = tuple( + (literal.group(1), literal.group(2)) + for call in _TITLED_CALL.finditer(source) + if (literal := _TITLE_LITERAL.match(source, call.end())) is not None + ) + return _SpecTitles( + literal=frozenset( + text for quote, text in titles if not (quote == "`" and "${" in text) + ), + patterns=tuple( + pattern + for quote, text in titles + if quote == "`" and "${" in text + if (pattern := _title_pattern(text)) is not None + ), + ) + + +def _unresolved_reason(cell: _UiCoveredCell, ui_dir: Path) -> str | None: + """Why this row no longer resolves against the suite, or None when it holds. + + The check is per declaration, never per file: an interpolated title elsewhere + in the spec is irrelevant unless it could itself have produced this title, so + renaming a literal test still fails even when a dynamic sibling sits beside + it.""" + spec = ui_dir / cell.spec + if not spec.is_file(): + return f"{cell.id}: spec {cell.spec} does not exist" + if _spec_titles(spec.read_text(encoding="utf-8")).covers(cell.test): + return None + return f"{cell.id}: {cell.spec} has no test titled {cell.test!r}" + + +def load_ui_declarations(path: Path = UI_DECLARATION_FILE) -> UiDeclarations: + """Cells the TypeScript Playwright suite declares it covers, each checked + against the spec and test title it names. A row whose spec or title no longer + exists is dropped from the numerator and reported, so renaming or deleting a + UI test cannot leave a cell counted forever. An id that resolves but is not in + the registry lands in the covered set and surfaces as an orphan marker, + exactly like a typo in a pytest marker.""" + if not path.is_file(): + return UiDeclarations(frozenset(), ()) + declaration = _UiDeclaration.model_validate(yaml.safe_load(path.read_text()) or {}) + checked = tuple( + (cell, _unresolved_reason(cell, path.parent)) for cell in declaration.covers + ) + return UiDeclarations( + ids=frozenset(cell.id for cell, reason in checked if reason is None), + unresolved=tuple( + reason for _, reason in checked if reason is not None + ), + ) class _CoversSink: @@ -71,6 +276,31 @@ def collect_covered_ids( return sink.covered_ids, sink.collection_errors +@dataclass(frozen=True, slots=True) +class CoveredIds: + ids: frozenset[str] + collection_errors: tuple[str, ...] + stale_ui_declarations: tuple[str, ...] + + +def covered_ids( + e2e_dir: Path = E2E_DIR, + ui_declaration: Path = UI_DECLARATION_FILE, +) -> CoveredIds: + """The numerator and everything that undermines it: every cell id declared in + the source, plus the ones only a live import can resolve, plus the TypeScript + suite's still-resolving declarations; every node that failed to parse or to + import; and every UI declaration that no longer points at a real test.""" + scanned, unparseable = scan_covers_markers(e2e_dir) + collected, import_errors = collect_covered_ids(e2e_dir) + ui = load_ui_declarations(ui_declaration) + return CoveredIds( + ids=scanned | collected | ui.ids, + collection_errors=(*unparseable, *import_errors), + stale_ui_declarations=ui.unresolved, + ) + + @dataclass(frozen=True, slots=True) class ModuleCoverage: module: str @@ -94,6 +324,7 @@ class CoverageReport: p0_gaps: tuple[str, ...] orphan_markers: tuple[str, ...] collection_errors: tuple[str, ...] + stale_ui_declarations: tuple[str, ...] = () @property def coverage_percent(self) -> float: @@ -122,6 +353,7 @@ def compute_coverage( cells: tuple[Cell, ...], covered: frozenset[str], collection_errors: tuple[str, ...] = (), + stale_ui_declarations: tuple[str, ...] = (), ) -> CoverageReport: p0_cells = tuple(c for c in cells if c.tier is Tier.P0) registry_ids = frozenset(c.id for c in cells) @@ -134,6 +366,7 @@ def compute_coverage( p0_gaps=tuple(sorted(c.id for c in p0_cells if c.id not in covered)), orphan_markers=tuple(sorted(covered - registry_ids)), collection_errors=collection_errors, + stale_ui_declarations=stale_ui_declarations, ) @@ -161,16 +394,25 @@ def render(report: CoverageReport) -> str: if report.orphan_markers else () ) + stale = ( + ( + f"\n{len(report.stale_ui_declarations)} UI declaration(s) no longer point at a " + f"real test and are not counted (reconcile tests/e2e/ui/coverage.yaml):\n " + + "\n ".join(report.stale_ui_declarations), + ) + if report.stale_ui_declarations + else () + ) warning = ( ( - f"\nWARNING: {len(report.collection_errors)} node(s) failed to import during " - f"collection, so coverage may undercount:\n " + f"\nWARNING: {len(report.collection_errors)} node(s) failed to parse or import, " + f"so coverage may undercount:\n " + "\n ".join(report.collection_errors), ) if report.collection_errors else () ) - return "\n".join((*lines, *orphans, *warning)) + return "\n".join((*lines, *orphans, *stale, *warning)) def _report_dict(report: CoverageReport) -> dict[str, object]: @@ -191,6 +433,7 @@ def _report_dict(report: CoverageReport) -> dict[str, object]: ], "orphan_markers": list(report.orphan_markers), "collection_errors": list(report.collection_errors), + "stale_ui_declarations": list(report.stale_ui_declarations), } @@ -237,6 +480,9 @@ def render_prometheus(report: CoverageReport) -> str: "# HELP litellm_e2e_coverage_collection_errors Pytest nodes that failed during collection.", "# TYPE litellm_e2e_coverage_collection_errors gauge", f"litellm_e2e_coverage_collection_errors {len(report.collection_errors)}", + "# HELP litellm_e2e_coverage_stale_ui_declarations UI coverage declarations whose spec or test title no longer exists.", + "# TYPE litellm_e2e_coverage_stale_ui_declarations gauge", + f"litellm_e2e_coverage_stale_ui_declarations {len(report.stale_ui_declarations)}", ] ) return "\n".join(lines) @@ -260,12 +506,24 @@ def render_loki(report: CoverageReport) -> str: return "\n".join(lines) -class _CliArgs(BaseModel): +class CliArgs(BaseModel): format: Literal["text", "json", "prometheus", "loki"] strict: bool fail_on_collection_errors: bool +def exit_code(report: CoverageReport, args: CliArgs) -> int: + """Non-zero when the run found something the operator asked to fail on: a claim + the tree does not support (an unknown cell id or a dead UI declaration) under + --strict, or a node whose cells could not be read under + --fail-on-collection-errors.""" + if args.strict and (report.orphan_markers or report.stale_ui_declarations): + return 1 + if args.fail_on_collection_errors and report.collection_errors: + return 1 + return 0 + + def main() -> int: parser = ArgumentParser() parser.add_argument( @@ -277,17 +535,22 @@ def main() -> int: parser.add_argument( "--strict", action="store_true", - help="Exit non-zero if markers outside the registry are found.", + help=( + "Exit non-zero if markers outside the registry are found, or if a UI " + "coverage declaration no longer points at a real Playwright test." + ), ) parser.add_argument( "--fail-on-collection-errors", action="store_true", help="Exit non-zero if pytest collection errors are found.", ) - args = _CliArgs.model_validate(vars(parser.parse_args())) + args = CliArgs.model_validate(vars(parser.parse_args())) cells = load_registry() - covered, errors = collect_covered_ids() - report = compute_coverage(cells, covered, errors) + covered = covered_ids() + report = compute_coverage( + cells, covered.ids, covered.collection_errors, covered.stale_ui_declarations + ) output = { "text": render, "json": render_json, @@ -295,11 +558,7 @@ def main() -> int: "loki": render_loki, }[args.format](report) print(output) # noqa: T201 # CLI entrypoint output - if args.strict and report.orphan_markers: - return 1 - if args.fail_on_collection_errors and report.collection_errors: - return 1 - return 0 + return exit_code(report, args) if __name__ == "__main__": diff --git a/tests/e2e/coverage_registry/llm_nonconversational.yaml b/tests/e2e/coverage_registry/llm_nonconversational.yaml index 1e49cb13538..15ea6999348 100644 --- a/tests/e2e/coverage_registry/llm_nonconversational.yaml +++ b/tests/e2e/coverage_registry/llm_nonconversational.yaml @@ -5,7 +5,6 @@ - {id: llm.embeddings.bedrock.basic.nonstream.works, module: llm, tier: P1, subject_endpoint: embeddings, route: bedrock_converse, capability: basic, streaming: nonstream, assertions: [works], source: "llms/bedrock/embed/embedding.py", rationale: "Bedrock Titan embeddings"} - {id: llm.embeddings.vertex.basic.nonstream.works, module: llm, tier: P1, subject_endpoint: embeddings, route: vertex, capability: basic, streaming: nonstream, assertions: [works], source: "vertex_embeddings/embedding_handler.py", rationale: "Vertex embeddings"} - {id: llm.embeddings.cohere.basic.nonstream.works, module: llm, tier: P1, subject_endpoint: embeddings, route: cohere, capability: basic, streaming: nonstream, assertions: [works], source: "llms/cohere/embed/handler.py", rationale: "Cohere embeddings"} -- {id: llm.embeddings.anthropic.basic.nonstream.works, module: llm, tier: P1, subject_endpoint: embeddings, route: anthropic, capability: basic, streaming: nonstream, assertions: [works], source: "llms/anthropic/chat/handler.py", rationale: "Anthropic vector API (verify support)"} - {id: llm.batches.openai.create.nonstream.works, module: llm, tier: P0, subject_endpoint: batches, route: openai, capability: basic, streaming: nonstream, assertions: [works], source: "test_batches_e2e.py", rationale: "Core batch create"} - {id: llm.batches.openai.retrieve.nonstream.works, module: llm, tier: P0, subject_endpoint: batches, route: openai, capability: basic, streaming: nonstream, assertions: [works], source: "test_batches_e2e.py", rationale: "Batch retrieve, id round-trip + status"} - {id: llm.batches.openai.cancel.nonstream.works, module: llm, tier: P0, subject_endpoint: batches, route: openai, capability: basic, streaming: nonstream, assertions: [works], source: "test_batches_e2e.py", rationale: "Batch cancel"} diff --git a/tests/e2e/coverage_registry/test_collector.py b/tests/e2e/coverage_registry/test_collector.py index 079ee215866..fc32d0a4030 100644 --- a/tests/e2e/coverage_registry/test_collector.py +++ b/tests/e2e/coverage_registry/test_collector.py @@ -7,16 +7,23 @@ duplicate ids. from __future__ import annotations +import json from pathlib import Path import pytest from coverage_registry.collector import ( + UI_DECLARATION_FILE, + CliArgs, compute_coverage, + covered_ids, + exit_code, + load_ui_declarations, render, render_json, render_loki, render_prometheus, + scan_covers_markers, ) from coverage_registry.registry import load_registry from coverage_registry.schema import ( @@ -24,6 +31,7 @@ from coverage_registry.schema import ( LlmCell, LlmEndpoint, LoggingCell, + MgmtCell, Tier, loki_module_label, ) @@ -192,3 +200,386 @@ def test_load_registry_rejects_duplicate_ids(tmp_path: Path) -> None: (tmp_path / "b.yaml").write_text(row) with pytest.raises(ValueError, match="duplicate cell ids"): load_registry(tmp_path) + + +def test_registry_has_no_anthropic_embeddings_row() -> None: + """Anthropic ships no embeddings API and litellm has no handler for one, so a + row asserting it can never pass. An unreachable row inflates the denominator + forever.""" + llm_cells = tuple(c for c in load_registry() if isinstance(c, LlmCell)) + assert not [ + c + for c in llm_cells + if c.subject_endpoint == "embeddings" and c.route == "anthropic" + ] + + +def _write(path: Path, source: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(source) + + +def _absent(tmp_path: Path) -> Path: + return tmp_path / "no-ui-declaration.yaml" + + +def test_marker_on_a_deselected_test_still_counts(tmp_path: Path) -> None: + """A cell is covered when a test declaring it exists. `tests/e2e/load/conftest.py` + deselects the weekly anomaly test unless an env var is set, and a conftest that + drops items must not be able to move the coverage number.""" + _write( + tmp_path / "conftest.py", + "import pytest\n\n\n" + "def pytest_collection_modifyitems(config, items):\n" + " dropped = [i for i in items if i.get_closest_marker('gated') is not None]\n" + " config.hook.pytest_deselected(items=dropped)\n" + " items[:] = [i for i in items if i.get_closest_marker('gated') is None]\n", + ) + _write( + tmp_path / "test_gated.py", + "import pytest\n\n\n" + "@pytest.mark.gated\n" + "@pytest.mark.covers('llm.gated.cell')\n" + "def test_gated() -> None:\n" + " pass\n", + ) + + result = covered_ids(tmp_path, _absent(tmp_path)) + ids, errors = result.ids, result.collection_errors + + assert "llm.gated.cell" in ids + assert errors == () + + +def test_marker_behind_an_optional_dependency_guard_still_counts(tmp_path: Path) -> None: + """`tests/e2e/mcp/test_mcp_chat_completion_oauth_e2e.py` sits behind + `pytest.importorskip`, so on a runner without the optional dep the module is + never imported and pytest reports no items for it. Its cells still exist.""" + _write( + tmp_path / "test_optional.py", + "import pytest\n\n" + "pytest.importorskip('a_package_that_is_not_installed_anywhere')\n\n\n" + "@pytest.mark.covers('mcp.optional.cell')\n" + "def test_optional() -> None:\n" + " pass\n", + ) + + result = covered_ids(tmp_path, _absent(tmp_path)) + ids, errors = result.ids, result.collection_errors + + assert "mcp.optional.cell" in ids + assert errors == () + + +def test_markers_built_at_import_time_still_count(tmp_path: Path) -> None: + """`batches/test_batches_e2e.py` computes its ids from a helper, so the source + text alone cannot see them; the collect-only pytest pass has to stay.""" + _write( + tmp_path / "test_dynamic.py", + "import pytest\n\n\n" + "def cells() -> tuple[str, ...]:\n" + " return ('llm.' + 'dynamic.cell',)\n\n\n" + "@pytest.mark.parametrize(\n" + " 'case', [pytest.param('a', marks=pytest.mark.covers(*cells()))]\n" + ")\n" + "def test_dynamic(case: str) -> None:\n" + " pass\n", + ) + + assert "llm.dynamic.cell" in covered_ids(tmp_path, _absent(tmp_path)).ids + + +def test_module_that_cannot_be_imported_is_reported_as_a_collection_error( + tmp_path: Path, +) -> None: + _write( + tmp_path / "test_broken.py", + "import a_package_that_is_not_installed_anywhere # noqa: F401\n", + ) + + errors = covered_ids(tmp_path, _absent(tmp_path)).collection_errors + + assert any("test_broken.py" in error for error in errors) + + +def test_unparseable_source_is_reported_rather_than_silently_dropped( + tmp_path: Path, +) -> None: + _write(tmp_path / "test_syntax.py", "def test_x(:\n") + + ids, errors = scan_covers_markers(tmp_path) + + assert ids == frozenset() + assert any("test_syntax.py" in error for error in errors) + + +_KEYS_SPEC = """import { test, expect } from "@playwright/test"; + +test.describe("Proxy Admin - Keys", () => { + test.use({ storageState: ADMIN_STORAGE_PATH }); + + test("Update key TPM and RPM limits", async ({ page }) => { + await page.getByRole("button", { name: "Save Changes" }).click(); + }); + + test.skip(!process.env.LITELLM_LICENSE, "proxy is running unlicensed"); +}); +""" + +_UI_CELL = MgmtCell( + id="mgmt.key.update.happy_path", + module="mgmt", + tier=Tier.P0, + assertions=("happy_path",), + source="t", + surface="ui", +) + + +def _ui_suite(tmp_path: Path, *, spec: str = _KEYS_SPEC, title: str) -> Path: + """A miniature ui/ suite: one spec plus a declaration claiming `title` in it.""" + _write(tmp_path / "ui" / "tests" / "proxy-admin" / "keys.spec.ts", spec) + declaration = tmp_path / "ui" / "coverage.yaml" + _write( + declaration, + "covers:\n" + " - id: mgmt.key.update.happy_path\n" + " spec: tests/proxy-admin/keys.spec.ts\n" + f" test: {json.dumps(title)}\n", + ) + return declaration + + +def test_ui_declaration_feeds_the_covered_set(tmp_path: Path) -> None: + """The TypeScript Playwright suite emits no pytest markers, so its cells reach + the numerator through a checked-in declaration file instead.""" + declaration = _ui_suite(tmp_path, title="Update key TPM and RPM limits") + + declared = load_ui_declarations(declaration) + + assert declared.ids == frozenset({"mgmt.key.update.happy_path"}) + assert declared.unresolved == () + assert compute_coverage((_UI_CELL,), covered_ids(tmp_path, declaration).ids).covered == 1 + + +def test_ui_declaration_stops_counting_when_its_test_is_renamed( + tmp_path: Path, +) -> None: + """Renaming or deleting the Playwright test must drop the cell out of the + numerator, so a claim here can never outlive the test that backs it.""" + declaration = _ui_suite(tmp_path, title="Update key TPM and RPM limits (renamed)") + + declared = load_ui_declarations(declaration) + + assert declared.ids == frozenset() + assert declared.unresolved == ( + "mgmt.key.update.happy_path: tests/proxy-admin/keys.spec.ts has no test " + "titled 'Update key TPM and RPM limits (renamed)'", + ) + report = compute_coverage( + (_UI_CELL,), declared.ids, stale_ui_declarations=declared.unresolved + ) + assert report.covered == 0 + assert "no test titled" in render(report) + + +def test_ui_declaration_stops_counting_when_its_spec_is_deleted( + tmp_path: Path, +) -> None: + declaration = tmp_path / "ui" / "coverage.yaml" + _write( + declaration, + "covers:\n" + " - id: mgmt.key.update.happy_path\n" + " spec: tests/proxy-admin/keys.spec.ts\n" + " test: Update key TPM and RPM limits\n", + ) + + declared = load_ui_declarations(declaration) + + assert declared.ids == frozenset() + assert declared.unresolved == ( + "mgmt.key.update.happy_path: spec tests/proxy-admin/keys.spec.ts does not exist", + ) + + +_SPEC_WITH_DYNAMIC_SIBLING = """import { test } from "@playwright/test"; + +test(`${segment}: sidebar nav and reload`, async ({ page }) => {}); + +test("Update key rate limits", async ({ page }) => {}); +""" + + +def test_a_dynamic_sibling_title_does_not_exempt_the_rest_of_the_spec( + tmp_path: Path, +) -> None: + """The exemption is per declaration, never per file. A renamed literal test + must still fail even when an interpolated title sits beside it in the same + spec, otherwise one templated title switches validation off for the file.""" + declaration = _ui_suite( + tmp_path, + spec=_SPEC_WITH_DYNAMIC_SIBLING, + title="Update key TPM and RPM limits", + ) + + declared = load_ui_declarations(declaration) + + assert declared.ids == frozenset() + assert declared.unresolved == ( + "mgmt.key.update.happy_path: tests/proxy-admin/keys.spec.ts has no test " + "titled 'Update key TPM and RPM limits'", + ) + + +def test_an_interpolated_title_is_matched_by_its_literal_segments( + tmp_path: Path, +) -> None: + """An interpolated title is still checked as far as it can be: the declared + title has to be one the template could actually have produced.""" + declaration = _ui_suite( + tmp_path, + spec=_SPEC_WITH_DYNAMIC_SIBLING, + title="api-keys: sidebar nav and reload", + ) + + declared = load_ui_declarations(declaration) + + assert declared.ids == frozenset({"mgmt.key.update.happy_path"}) + assert declared.unresolved == () + + +def test_a_title_the_template_cannot_produce_is_rejected(tmp_path: Path) -> None: + declaration = _ui_suite( + tmp_path, + spec=_SPEC_WITH_DYNAMIC_SIBLING, + title="api-keys: some other flow entirely", + ) + + assert load_ui_declarations(declaration).ids == frozenset() + + +def test_a_wholly_interpolated_title_backs_no_declaration(tmp_path: Path) -> None: + """A title that is nothing but interpolation would match anything, so it is + not allowed to satisfy a row; the contributor has to give the test a title + with something literal in it.""" + spec = ( + 'import { test } from "@playwright/test";\n\n' + "test(`${title}`, async ({ page }) => {});\n" + ) + declaration = _ui_suite(tmp_path, spec=spec, title="Update key TPM and RPM limits") + + assert load_ui_declarations(declaration).ids == frozenset() + + +def test_a_commented_out_test_backs_no_declaration(tmp_path: Path) -> None: + """Commenting a test out while debugging and forgetting to restore it leaves + the title sitting in the source. It must not keep the cell counted.""" + spec = ( + 'import { test } from "@playwright/test";\n\n' + '// test("Update key TPM and RPM limits", async ({ page }) => {});\n' + ) + declaration = _ui_suite(tmp_path, spec=spec, title="Update key TPM and RPM limits") + + assert load_ui_declarations(declaration).ids == frozenset() + + +def test_a_block_commented_test_backs_no_declaration(tmp_path: Path) -> None: + spec = ( + 'import { test } from "@playwright/test";\n\n' + "/*\n" + 'test("Update key TPM and RPM limits", async ({ page }) => {});\n' + "*/\n" + ) + declaration = _ui_suite(tmp_path, spec=spec, title="Update key TPM and RPM limits") + + assert load_ui_declarations(declaration).ids == frozenset() + + +def test_a_title_containing_a_url_still_validates(tmp_path: Path) -> None: + """Stripping comments must not touch `//` inside a string. A false negative + here would be worse than the commented-out case it guards against.""" + title = "redirects to https://example.com/ui after login" + spec = ( + 'import { test } from "@playwright/test";\n\n' + f'test("{title}", async ({{ page }}) => {{}});\n' + ) + declaration = _ui_suite(tmp_path, spec=spec, title=title) + + declared = load_ui_declarations(declaration) + + assert declared.ids == frozenset({"mgmt.key.update.happy_path"}) + assert declared.unresolved == () + + +def test_a_comment_does_not_swallow_the_titles_after_it(tmp_path: Path) -> None: + """An apostrophe in a comment must not open a string that eats the rest of the + file; the real specs are full of comments like this one.""" + spec = ( + 'import { test } from "@playwright/test";\n\n' + "// antd's dropdown renders off-viewport during the open animation\n" + "/* the modal's footer button text varies between versions */\n" + 'test("Update key TPM and RPM limits", async ({ page }) => {});\n' + ) + declaration = _ui_suite(tmp_path, spec=spec, title="Update key TPM and RPM limits") + + assert load_ui_declarations(declaration).ids == frozenset( + {"mgmt.key.update.happy_path"} + ) + + +def test_a_title_assembled_from_variables_backs_no_declaration( + tmp_path: Path, +) -> None: + spec = ( + 'import { test } from "@playwright/test";\n\n' + "test(TITLE, async ({ page }) => {});\n" + ) + declaration = _ui_suite(tmp_path, spec=spec, title="Update key TPM and RPM limits") + + assert load_ui_declarations(declaration).ids == frozenset() + + +def test_stale_ui_declaration_fails_strict(tmp_path: Path) -> None: + """A dead declaration and an unknown cell id are the same failure: the tree no + longer supports what the registry claims. Both must fail --strict.""" + declaration = _ui_suite(tmp_path, title="Update key TPM and RPM limits (renamed)") + declared = load_ui_declarations(declaration) + report = compute_coverage( + (_UI_CELL,), declared.ids, stale_ui_declarations=declared.unresolved + ) + strict = CliArgs(format="text", strict=True, fail_on_collection_errors=False) + lenient = CliArgs(format="text", strict=False, fail_on_collection_errors=False) + + assert exit_code(report, strict) == 1 + assert exit_code(report, lenient) == 0 + + +def test_ui_declaration_id_outside_the_registry_is_an_orphan(tmp_path: Path) -> None: + declaration = _ui_suite(tmp_path, title="Update key TPM and RPM limits") + _write( + declaration, + "covers:\n" + " - id: mgmt.key.typo.happy_path\n" + " spec: tests/proxy-admin/keys.spec.ts\n" + " test: Update key TPM and RPM limits\n", + ) + + report = compute_coverage( + (_llm("llm.a", Tier.P0),), load_ui_declarations(declaration).ids + ) + + assert report.covered == 0 + assert report.orphan_markers == ("mgmt.key.typo.happy_path",) + + +def test_missing_ui_declaration_claims_nothing(tmp_path: Path) -> None: + declared = load_ui_declarations(_absent(tmp_path)) + assert (declared.ids, declared.unresolved) == (frozenset(), ()) + + +def test_checked_in_ui_declaration_resolves_and_names_real_registry_cells() -> None: + registry_ids = frozenset(c.id for c in load_registry()) + declared = load_ui_declarations(UI_DECLARATION_FILE) + assert declared.unresolved == () + assert declared.ids <= registry_ids diff --git a/tests/e2e/ui/coverage.yaml b/tests/e2e/ui/coverage.yaml new file mode 100644 index 00000000000..cc212754459 --- /dev/null +++ b/tests/e2e/ui/coverage.yaml @@ -0,0 +1,40 @@ +# Coverage-registry cells this Playwright suite covers. +# +# Every other e2e suite is Python and declares coverage with +# @pytest.mark.covers(""). This suite is TypeScript, so it declares the +# same thing here and tests/e2e/coverage_registry/collector.py unions these ids +# into the covered set. +# +# To add a row: write the UI test first, then name the registry cell it proves, +# the spec file it lives in, and the test title. The id must exist in +# tests/e2e/coverage_registry/*.yaml; a typo lands in the collector's orphan +# marker list exactly like a typo in a pytest marker, and `--strict` fails on it. +# Claim a cell only when the spec actually asserts that behavior. +# +# `spec` is relative to this file and `test` is the Playwright test title, and +# the collector resolves both against the tree on every run. Rename or delete +# that test and the row stops counting and fails `--strict`, so a claim here +# cannot outlive the test that backs it. +# +# Commented-out code does not count as a test, so a row whose test is commented +# out fails the same way a deleted one does. +# +# An interpolated title such as test(`${role} sidebar`) is checked against its +# literal segments, so write out the title as it renders (e.g. "admin sidebar"). +# The check is per row, never per file: an interpolated title elsewhere in the +# spec does not exempt your row. A title with no literal text at all, or one +# assembled from variables, cannot back a row; give that test a title with +# something literal in it. +covers: + - id: mgmt.key.update.happy_path + spec: tests/proxy-admin/keys.spec.ts + test: Update key TPM and RPM limits + +# Not claimed yet: +# +# mgmt.key.generate.happy_path - the registry row scopes this to SSO-driven key +# generation ("SSO-driven key gen (UI path)", source ui_sso.py). This suite +# creates keys through the dashboard, but every role logs in with +# username/password (globalSetup.ts), so no test drives the SSO path. Claim it +# once a spec exercises SSO login, or retarget the registry row at plain +# dashboard key creation and claim it then.