diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 3aec104c861..d2f3395e730 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -15,14 +15,13 @@ shared fixtures build on it. import functools import sys -from collections.abc import Generator, Iterator +from collections.abc import Iterator from pathlib import Path import pytest import requests from e2e_config import CONTROL_PLANE_BASE_URL, PROXY_BASE_URL -from e2e_result_reporter import covers_from_item, format_e2e_result_line, result_from_pytest from lifecycle import GatewayProvider, ResourceManager @@ -40,6 +39,46 @@ def pytest_configure(config: pytest.Config) -> None: ) +def _package_from_nodeid(nodeid: str) -> str: + """Top-level suite package under tests/e2e/, or 'root' for top-level files. + + Pytest nodeids are relative to the invocation cwd. Repo-root runs look like + `tests/e2e/logging/...`; suite-cwd runs look like `logging/...`. Strip the + `tests/e2e` prefix so package is the suite dir either way. + """ + path_part = nodeid.split("::", 1)[0].replace("\\", "/") + raw = tuple(p for p in path_part.split("/") if p and p != ".") + parts = raw[2:] if len(raw) >= 3 and raw[0] == "tests" and raw[1] == "e2e" else raw + if len(parts) <= 1: + return "root" + return parts[0] + + +def _covers_from_item(item: pytest.Item) -> tuple[str, ...]: + """Read @pytest.mark.covers cell ids off a pytest Item, order-preserving.""" + marker_args: tuple[tuple[object, ...], ...] = tuple(marker.args for marker in item.iter_markers(name="covers")) + return tuple(dict.fromkeys(arg for args in marker_args for arg in args if isinstance(arg, str) and arg)) + + +def _result_properties(item: pytest.Item) -> tuple[tuple[str, str], ...]: + """The only custom signal a standard reporter cannot derive on its own: the + normalized suite package and the coverage-registry cell ids this test covers.""" + return ( + ("package", _package_from_nodeid(item.nodeid)), + ("covers", ",".join(_covers_from_item(item))), + ) + + +def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: + """Attach package + covers to every item's user_properties so a standard + reporter (`--junitxml`) captures them per test, on every outcome including + skips and setup errors. Downstream (Loki/Grafana) reads outcome and duration + from the standard artifact and these two properties for package rollups and + coverage drill-down; no bespoke log line is emitted.""" + for item in items: + item.user_properties.extend(_result_properties(item)) + + def _liveness_reason(label: str, base_url: str) -> str | None: """None if `base_url` answers its liveness probe, else a failure reason.""" try: @@ -86,30 +125,6 @@ def pytest_runtest_call(item: pytest.Item) -> None: item.session.stash[_E2E_TEST_RAN] = True -@pytest.hookimpl(wrapper=True, tryfirst=True) -def pytest_runtest_makereport( - item: pytest.Item, call: pytest.CallInfo[object] -) -> Generator[None, pytest.TestReport, pytest.TestReport]: - """Emit one structured E2E_RESULT line per finished test for Loki/Grafana. - - Status-history panels should aggregate by package (and optional covers), not - scrape pytest progress basenames. See e2e_result_reporter.py. - """ - report = yield - result = result_from_pytest( - nodeid=str(report.nodeid), - when=str(report.when), - failed=bool(report.failed), - skipped=bool(report.skipped), - passed=bool(report.passed), - duration_seconds=float(report.duration), - covers=covers_from_item(item), - ) - if result is not None: - print(format_e2e_result_line(result), flush=True) - return report - - def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None: """Once the whole e2e session is done (all suites), truncate the spend logs so the DB doesn't accumulate test rows. Sessions where no e2e test body ran leave diff --git a/tests/e2e/e2e_result_reporter.py b/tests/e2e/e2e_result_reporter.py deleted file mode 100644 index 22f7581818f..00000000000 --- a/tests/e2e/e2e_result_reporter.py +++ /dev/null @@ -1,144 +0,0 @@ -"""Structured e2e result lines for Loki / Grafana status history. - -Pytest progress lines are a bad dashboard source: they only expose file basenames, -break under quiet modes, and force status-history rows to explode with suite growth. - -Each finished test emits one logfmt line: - - E2E_RESULT package=logging file=test_langfuse_e2e.py outcome=failed - duration_ms=1234 node_id=logging/test_langfuse_e2e.py::TestX::test_y - covers=logging.langfuse.team.success - -Grafana package status-history queries max(fail) by package over E2E_RESULT lines. -Drill-down uses node_id / covers in Explore, not status-history cardinality. -""" - -from __future__ import annotations - -from collections.abc import Iterable, Sequence -from dataclasses import dataclass -from pathlib import Path -from typing import Literal, Protocol, runtime_checkable - -Outcome = Literal["passed", "failed", "error", "skipped"] - - -@dataclass(frozen=True, slots=True) -class E2EResult: - package: str - file: str - outcome: Outcome - duration_ms: int - node_id: str - covers: tuple[str, ...] - - -@runtime_checkable -class _MarkerArgs(Protocol): - args: Sequence[object] - - -@runtime_checkable -class _ItemWithCovers(Protocol): - def iter_markers(self, name: str) -> Iterable[object]: ... - - -def package_from_nodeid(nodeid: str) -> str: - """Top-level suite package under tests/e2e/, or 'root' for top-level files. - - Pytest nodeids are relative to the invocation cwd. Repo-root runs look like - `tests/e2e/logging/...`; suite-cwd runs look like `logging/...`. Strip the - `tests/e2e` prefix so package is the suite dir either way. - """ - path_part = nodeid.split("::", 1)[0].replace("\\", "/") - parts = tuple(p for p in path_part.split("/") if p and p != ".") - if len(parts) >= 3 and parts[0] == "tests" and parts[1] == "e2e": - parts = parts[2:] - if len(parts) <= 1: - return "root" - return parts[0] - - -def file_from_nodeid(nodeid: str) -> str: - path_part = nodeid.split("::", 1)[0].replace("\\", "/") - return Path(path_part).name - - -def covers_from_item(item: object) -> tuple[str, ...]: - """Read @pytest.mark.covers cell ids from a pytest Item.""" - if not isinstance(item, _ItemWithCovers): - return () - return tuple( - dict.fromkeys( - arg - for marker in item.iter_markers(name="covers") - if isinstance(marker, _MarkerArgs) - for arg in marker.args - if isinstance(arg, str) and arg - ) - ) - - -def outcome_from_report(when: str, failed: bool, skipped: bool, passed: bool) -> Outcome | None: - """Map pytest TestReport fields to a terminal outcome. None if not final.""" - if when == "setup" and skipped: - return "skipped" - if when == "setup" and failed: - return "error" - if when != "call": - return None - if skipped: - return "skipped" - if failed: - return "failed" - if passed: - return "passed" - return "failed" - - -def _logfmt_escape(value: str) -> str: - if value == "": - return '""' - needs_quote = any(ch.isspace() or ch in "\"=\\" for ch in value) - if not needs_quote: - return value - escaped = value.replace("\\", "\\\\").replace('"', '\\"') - return f'"{escaped}"' - - -def format_e2e_result_line(result: E2EResult) -> str: - covers = ",".join(result.covers) - fields = ( - ("package", result.package), - ("file", result.file), - ("outcome", result.outcome), - ("duration_ms", str(result.duration_ms)), - ("node_id", result.node_id), - ("covers", covers), - ) - body = " ".join(f"{key}={_logfmt_escape(value)}" for key, value in fields) - return f"E2E_RESULT {body}" - - -def result_from_pytest( - *, - nodeid: str, - when: str, - failed: bool, - skipped: bool, - passed: bool, - duration_seconds: float, - covers: tuple[str, ...] = (), -) -> E2EResult | None: - outcome = outcome_from_report(when=when, failed=failed, skipped=skipped, passed=passed) - if outcome is None: - return None - duration_ms = max(0, int(round(duration_seconds * 1000))) - return E2EResult( - package=package_from_nodeid(nodeid), - file=file_from_nodeid(nodeid), - outcome=outcome, - duration_ms=duration_ms, - node_id=nodeid, - covers=covers, - ) diff --git a/tests/e2e/grafana/status_history_panels.md b/tests/e2e/grafana/status_history_panels.md index f8cda509c63..2b6ac2960e0 100644 --- a/tests/e2e/grafana/status_history_panels.md +++ b/tests/e2e/grafana/status_history_panels.md @@ -6,18 +6,54 @@ The old **test suite status history** panel scraped pytest progress lines and grouped by **file basename** (`test_foo.py`). That does not scale: multi-class files collapse to one bit, and full `node_id` cardinality melts status-history. -## Emitter +## Artifact -After each test finishes, `tests/e2e/conftest.py` prints one logfmt line: +The run emits a standard pytest JUnit XML report instead of a bespoke +`E2E_RESULT` log line. Point pytest at a file with `--junitxml`: ``` -E2E_RESULT package=logging file=test_langfuse_e2e.py outcome=failed duration_ms=1500 node_id="logging/..." covers=cell.id +uv run pytest tests/e2e --junitxml=e2e-report.xml ``` +Each finished test is one ``; pytest fills in `classname`, `name`, +`time` (duration in seconds), and a child ``, ``, or `` +element for a non-passing outcome (a bare `` is a pass). The two custom +signals ride along as `` entries, attached to every test in +`conftest.py::pytest_collection_modifyitems`: + +```xml + + + + + + ... + +``` + +- `package`: top-level suite dir under `tests/e2e/` (`root` for top-level files), + normalized so a repo-root run and a suite-cwd run agree. +- `covers`: comma-joined `@pytest.mark.covers` cell ids (empty string when none). + +Outcome maps from the child element: `` -> failed, `` -> error, +`` -> skipped, none -> passed. Duration is the `time` attribute. + +## Shipping to Loki + +JUnit XML is not line-based, so it is not tailed directly the way the old logfmt +line was. Ship it with a thin converter in the e2e job (Grafana Agent / promtail +cannot parse XML on their own): after the run, walk `e2e-report.xml` and print one +logfmt line per `` to the pod's stdout, which the existing scrape +already forwards to Loki. A `xmltodict` / `xml.etree` one-liner in the job is +enough; the emitted line should carry `package`, `covers`, `outcome`, +`duration_ms`, and `node_id` (`classname::name`) so the queries below keep +working unchanged. Building that converter and its scrape config is +infra-side and out of scope for this repo change. + ## Panel: package status history (replace panel 11) -**Type:** Status history -**Interval:** 15m (or 1h for multi-day ranges) +**Type:** Status history +**Interval:** 15m (or 1h for multi-day ranges) **Description:** Per top-level package under `tests/e2e/`: red if any test failed or errored in the bucket. ```logql @@ -34,7 +70,7 @@ max by (package) ( ) ``` -Value mappings: `0` → Pass (green), `1` → Fail (red). +Value mappings: `0` -> Pass (green), `1` -> Fail (red). If `service_name` is missing on older scrapes, use: @@ -50,11 +86,11 @@ instead of `{service_name="litellm-e2e"}`. {service_name="litellm-e2e"} |= "E2E_RESULT" | logfmt | outcome=~"failed|error" ``` -Show fields: `package`, `file`, `node_id`, `covers`, `duration_ms`. +Show fields: `package`, `covers`, `node_id`, `duration_ms`. ## Panel (optional): filter by package variable -Dashboard variable `package` (custom or from label_values on E2E_RESULT): +Dashboard variable `package` (custom or from label_values on the shipped lines): ```logql {service_name="litellm-e2e"} |= "E2E_RESULT" | logfmt | package=`$package` | outcome=~"failed|error" @@ -63,4 +99,4 @@ Dashboard variable `package` (custom or from label_values on E2E_RESULT): ## Do not - Put full `node_id` as the status-history series key (cardinality). -- Rely on `::S+ PASSED` progress regex as the primary signal once E2E_RESULT is live. +- Rely on `::S+ PASSED` progress regex as the primary signal.