Record each e2e test's source location in the JUnit report

The JUnit report is the only thing that leaves the e2e run, and it says
where a test's results came from but never where its code lives. A reader
looking at `test_cell_claimed_only_by_a_skipped_test_is_uncovered` on the
status page has a name and nothing else -- no file, no line, no way to
reach the source short of grepping the repo by hand.

Pytest knows the location; the report format loses it. The `xunit1` family
wrote `file=` and `line=` onto every `<testcase>`, and the `xunit2` default
this suite runs on drops both. Switching families back would change the
document for every consumer of the same XML -- the Buildkite Test Engine
upload and the Loki pipeline included -- so add the location the way this
suite already adds `package` and `covers`: as a `<property>`, which is
purely additive.

`source` is repo-relative and one-based (`tests/e2e/a2a/test_x.py:41`), so
a consumer can build a link without knowing how pytest was started. That
takes normalizing the two launch shapes -- the runner image runs from its
own copy at /app/e2e, a developer runs from the repo root -- which is the
same normalization `package_from_nodeid` was already doing in reverse, now
factored into `suite_parts` so the two cannot drift apart. Paths that
escape the suite, and tests pytest reports no line for, emit an empty
string: a test with no link beats a link that 404s.

Claude-Session: https://claude.ai/code/session_017dTKXwJkzhtVLzDhePHsKG

tests/e2e/junit_properties.py

@@ -2,10 +2,20 @@
 
 The e2e suite ships results to Loki/Grafana from a standard pytest JUnit report
 (`--junitxml=e2e-report.xml`), not a bespoke log line. JUnit already records
-outcome, duration, and node id for every `<testcase>`; the only signals it cannot
-derive on its own are the normalized suite package and the coverage-registry cell
-ids a test covers. Those ride along as JUnit `<property>` entries via each item's
-`user_properties`, attached in `conftest.py::pytest_collection_modifyitems`.
+outcome, duration, and node id for every `<testcase>`; the signals it cannot
+derive on its own are the normalized suite package, the coverage-registry cell
+ids a test covers, and where the test's source lives. Those ride along as JUnit
+`<property>` entries via each item's `user_properties`, attached in
+`conftest.py::pytest_collection_modifyitems`.
+
+`source` is here because of a reporter limitation rather than a missing pytest
+fact. Pytest knows every test's file and line, and its `xunit1` report family
+wrote them as `file=` / `line=` attributes on `<testcase>`. The default `xunit2`
+family -- pytest's since 6.0, and this suite's, since pytest.ini names no family
+-- drops both. Switching families to get them back would change the document
+shape for every consumer of the same XML, the Buildkite Test Engine upload and
+the Loki pipeline included; a property is additive, so nothing that reads the
+report today sees a difference.
 """
 
 from __future__ import annotations
@@ -14,22 +24,66 @@ from collections.abc import Iterable
 
 import pytest
 
+# This module's own directory, relative to the repo root. Hardcoded because it
+# cannot be discovered at runtime: the e2e runner image copies tests/e2e/ to
+# /app/e2e and runs pytest from there, so no ancestor of this file names the
+# suite's place in the litellm tree. Moving tests/e2e/ means editing this line,
+# and test_junit_properties.py fails from a checkout until you do.
+SUITE_ROOT = "tests/e2e"
+
+
+def suite_parts(path_part: str) -> tuple[str, ...]:
+    """Path components of a suite file, relative to tests/e2e, either way it ran.
+
+    Pytest reports paths relative to its rootdir, which moves with the
+    invocation: a repo-root run gives `tests/e2e/logging/test_x.py`, a suite-cwd
+    run (what the runner image does) gives `logging/test_x.py`. Strip the
+    `tests/e2e` prefix when present so both collapse to the same components.
+    """
+    raw = tuple(p for p in path_part.replace("\\", "/").split("/") if p and p != ".")
+    return raw[2:] if len(raw) >= 3 and raw[0] == "tests" and raw[1] == "e2e" else raw
+
 
 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
+    """Top-level suite package under tests/e2e/, or 'root' for top-level files."""
+    parts = suite_parts(nodeid.split("::", 1)[0])
     if len(parts) <= 1:
         return "root"
     return parts[0]
 
 
+def source_from_location(path: str, lineno: int | None) -> str:
+    """Repo-relative `path:line` for a test, or '' when nothing is linkable.
+
+    `pytest.Item.location` supplies a rootdir-relative path and a ZERO-based
+    line number, and neither travels as-is. The path is re-rooted at SUITE_ROOT
+    so consumers never have to know how pytest was started, and the line is
+    emitted ONE-based, matching editors, tracebacks, and code hosts (GitHub's
+    `#L41` is the file's 41st line). A decorated test anchors at its first
+    decorator, which is where pytest reports it and which puts the marks and the
+    `def` on screen together.
+
+    Returns '' rather than a guess when pytest reports no line, or when the path
+    escapes the suite root (absolute, or reaching upward): a test that renders
+    without a link is a smaller failure than one that links somewhere wrong.
+    """
+    if lineno is None:
+        return ""
+    normalized = path.replace("\\", "/")
+    if normalized.startswith("/") or ".." in normalized.split("/"):
+        return ""
+    parts = suite_parts(normalized)
+    if not parts:
+        return ""
+    return f"{'/'.join((SUITE_ROOT, *parts))}:{lineno + 1}"
+
+
+def source_from_item(item: pytest.Item) -> str:
+    """Read the repo-relative `path:line` off a pytest Item's reported location."""
+    path, lineno, _ = item.location
+    return source_from_location(path, lineno)
+
+
 def dedupe_covers(marker_args: Iterable[tuple[object, ...]]) -> tuple[str, ...]:
     """Flatten @pytest.mark.covers arg lists into unique, order-preserving cell
     ids, dropping anything that is not a non-empty string."""
@@ -43,10 +97,12 @@ def covers_from_item(item: pytest.Item) -> tuple[str, ...]:
 
 def result_properties(item: pytest.Item) -> tuple[tuple[str, str], ...]:
     """The custom signals a standard reporter cannot derive: the normalized suite
-    package and the comma-joined coverage-registry cell ids this test covers."""
+    package, the comma-joined coverage-registry cell ids this test covers, and the
+    repo-relative `path:line` its source sits at."""
     return (
         ("package", package_from_nodeid(item.nodeid)),
         ("covers", ",".join(covers_from_item(item))),
+        ("source", source_from_item(item)),
     )
 
 

tests/e2e/test_junit_properties.py

@@ -0,0 +1,145 @@
+"""Harness coverage for the custom JUnit properties.
+
+No proxy and no ``e2e`` marker. Pins the two normalizations that have to agree
+about where a suite file lives -- ``package_from_nodeid`` (strip the suite root)
+and ``source_from_location`` (re-root at it) -- across both ways the suite is
+launched, plus the one-based line offset and the refusal to emit a path that
+escapes the suite. The consumers of these properties are the Loki/Grafana
+rollups and, for ``source``, the status page's per-test links to GitHub.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+from junit_properties import (
+    SUITE_ROOT,
+    attach_result_properties,
+    dedupe_covers,
+    package_from_nodeid,
+    result_properties,
+    source_from_location,
+    suite_parts,
+)
+
+
+class FakeMarker:
+    def __init__(self, name: str, *args: object) -> None:
+        self.name = name
+        self.args = args
+
+
+class FakeItem:
+    """The three attributes junit_properties reads off a pytest Item."""
+
+    def __init__(
+        self, nodeid: str, location: tuple[str, int | None, str], markers: tuple[FakeMarker, ...] = ()
+    ) -> None:
+        self.nodeid = nodeid
+        self.location = location
+        self.user_properties: list[tuple[str, str]] = []
+        self._markers = markers
+
+    def iter_markers(self, name: str):
+        return (marker for marker in self._markers if marker.name == name)
+
+
+def repo_root() -> Path | None:
+    """The litellm checkout above this file, or None when there isn't one."""
+    return next((p for p in Path(__file__).resolve().parents if (p / ".git").exists()), None)
+
+
+class TestSuiteParts:
+    @pytest.mark.parametrize(
+        "path",
+        ["logging/test_x.py", "tests/e2e/logging/test_x.py", "./logging/test_x.py", "tests\\e2e\\logging\\test_x.py"],
+    )
+    def test_both_invocation_shapes_collapse_to_the_same_components(self, path: str) -> None:
+        """A repo-root run and a suite-cwd run report the same file differently;
+        every downstream signal has to see one spelling."""
+        assert suite_parts(path) == ("logging", "test_x.py")
+
+    def test_top_level_suite_file_keeps_its_single_component(self) -> None:
+        assert suite_parts("tests/e2e/test_fixture_mode.py") == ("test_fixture_mode.py",)
+
+
+class TestPackageFromNodeid:
+    @pytest.mark.parametrize(
+        ("nodeid", "expected"),
+        [
+            ("logging/test_x.py::TestFoo::test_bar", "logging"),
+            ("tests/e2e/logging/test_x.py::TestFoo::test_bar", "logging"),
+            ("quota_management/spend_tracking/test_x.py::test_bar", "quota_management"),
+            ("test_fixture_mode.py::TestParseFixtureMode::test_known_values_normalize", "root"),
+            ("tests/e2e/test_fixture_mode.py::test_bar", "root"),
+        ],
+    )
+    def test_package_is_the_first_dir_under_the_suite_root(self, nodeid: str, expected: str) -> None:
+        assert package_from_nodeid(nodeid) == expected
+
+
+class TestSourceFromLocation:
+    @pytest.mark.parametrize("path", ["a2a/test_a2a_agent_e2e.py", "tests/e2e/a2a/test_a2a_agent_e2e.py"])
+    def test_path_is_repo_relative_however_pytest_was_started(self, path: str) -> None:
+        assert source_from_location(path, 40) == "tests/e2e/a2a/test_a2a_agent_e2e.py:41"
+
+    def test_line_is_emitted_one_based(self) -> None:
+        """pytest.Item.location counts from 0; editors, tracebacks and GitHub's
+        #L anchor all count from 1, and an off-by-one lands on the decorator."""
+        assert source_from_location("a2a/test_x.py", 0) == "tests/e2e/a2a/test_x.py:1"
+
+    def test_top_level_suite_file_sits_directly_under_the_suite_root(self) -> None:
+        assert source_from_location("test_fixture_mode.py", 39) == "tests/e2e/test_fixture_mode.py:40"
+
+    @pytest.mark.parametrize(
+        ("path", "lineno"),
+        [
+            ("a2a/test_x.py", None),
+            ("/app/e2e/a2a/test_x.py", 40),
+            ("../conftest.py", 40),
+            ("", 40),
+        ],
+    )
+    def test_nothing_linkable_yields_empty_rather_than_a_guess(self, path: str, lineno: int | None) -> None:
+        """A test that renders without a link is a smaller failure than one whose
+        link 404s or points into another repo's file."""
+        assert source_from_location(path, lineno) == ""
+
+
+class TestResultProperties:
+    def test_every_test_carries_package_covers_and_source(self) -> None:
+        item = FakeItem(
+            "logging/test_x.py::TestFoo::test_bar",
+            ("logging/test_x.py", 40, "TestFoo.test_bar"),
+            (FakeMarker("covers", "LOG-1", "LOG-2"),),
+        )
+        assert result_properties(item) == (
+            ("package", "logging"),
+            ("covers", "LOG-1,LOG-2"),
+            ("source", "tests/e2e/logging/test_x.py:41"),
+        )
+
+    def test_attach_is_idempotent(self) -> None:
+        """Collection can run the hook more than once; a second pass must not
+        double the <property> entries in the report."""
+        item = FakeItem("logging/test_x.py::test_bar", ("logging/test_x.py", 40, "test_bar"))
+        attach_result_properties(item)
+        attach_result_properties(item)
+        assert [name for name, _ in item.user_properties] == ["package", "covers", "source"]
+
+
+class TestSuiteRoot:
+    def test_suite_root_names_this_file_s_real_home(self) -> None:
+        """SUITE_ROOT is hardcoded because the runner image has no repo to read it
+        from. Where there IS a checkout, prove the constant still points at us --
+        otherwise a moved tests/e2e/ ships links that 404."""
+        root = repo_root()
+        if root is None:
+            pytest.skip("no checkout above this file (the runner image copies tests/e2e/ to /app/e2e)")
+        assert (root / SUITE_ROOT / Path(__file__).name).resolve() == Path(__file__).resolve()
+
+
+class TestDedupeCovers:
+    def test_ids_are_unique_order_preserving_and_non_empty_strings(self) -> None:
+        assert dedupe_covers([("A", "B"), ("B", ""), ("C", 7)]) == ("A", "B", "C")