diff --git a/tests/code_coverage_tests/test_e2e_junit_report.py b/tests/code_coverage_tests/test_e2e_junit_report.py index 4d9faaf21b6..f98cc25a2d1 100644 --- a/tests/code_coverage_tests/test_e2e_junit_report.py +++ b/tests/code_coverage_tests/test_e2e_junit_report.py @@ -28,6 +28,7 @@ from typing import Final from xml.etree import ElementTree import pytest +from pydantic import TypeAdapter SUITE_DIR: Final = Path(__file__).resolve().parents[1] / "e2e" CHILD_TIMEOUT_SECONDS: Final = 180 @@ -147,22 +148,61 @@ def test_dies_in_a_module_scoped_fixture(identity: None) -> None: assert identity is None """ +FAILED_PHASE_SUITE: Final = """ +import pytest +from e2e_metadata import step + + +@step("open the consent page") +def open_consent() -> None: + raise RuntimeError("consent page timed out") + + +@pytest.mark.mcp_oauth_live +def test_oauth_dies_on_consent() -> None: + open_consent() + + +def test_plain_dies_on_consent() -> None: + open_consent() +""" + +REPORT_SPY_PLUGIN: Final = """ +import json +from pathlib import Path + +import pytest + +SEEN = Path(__file__).with_name("failed-reports.jsonl") + + +def pytest_runtest_logreport(report: pytest.TestReport) -> None: + if report.failed: + steps = [value for name, value in report.user_properties if name == "step"] + with SEEN.open("a") as out: + out.write(json.dumps([report.nodeid.split("::")[-1], steps]) + "\\n") +""" + Properties = tuple[tuple[str, str], ...] +FailedReport: Final = TypeAdapter(tuple[str, tuple[str, ...]]) def write_suite(directory: Path, modules: Mapping[str, str]) -> None: """Lay a child suite out in ``directory``, with an ini file of its own. The ini pins the child's rootdir to the tmp dir wherever that lives, and its - ``pythonpath`` is what makes tests/e2e's conftest.py, and the harness - modules the child suite imports, importable under ``-I``. + ``pythonpath`` is what makes tests/e2e's conftest.py, the harness modules + the child suite imports, and any plugin laid out beside it importable under ``-I``. """ - _ = (directory / "pytest.ini").write_text(f"[pytest]\npythonpath = {shlex.quote(str(SUITE_DIR))}\n") + paths: Final = " ".join(shlex.quote(str(path)) for path in (SUITE_DIR, directory)) + _ = (directory / "pytest.ini").write_text(f"[pytest]\npythonpath = {paths}\n") for name, source in modules.items(): _ = (directory / name).write_text(source) -def run_child_pytest(suite: Path, *args: str) -> subprocess.CompletedProcess[str]: +def run_child_pytest( + suite: Path, *args: str, env: Mapping[str, str] = MappingProxyType({}) +) -> subprocess.CompletedProcess[str]: """Run pytest over ``suite`` in a fresh interpreter, hooked up like the live suite. ``-p conftest`` registers tests/e2e's conftest.py as a plugin, since a @@ -178,7 +218,7 @@ def run_child_pytest(suite: Path, *args: str) -> subprocess.CompletedProcess[str return subprocess.run( [sys.executable, "-I", "-m", "pytest", "-p", "conftest", "-p", "no:cacheprovider", *args, str(suite)], cwd=suite, - env=inherited, + env={**inherited, **env}, capture_output=True, text=True, timeout=CHILD_TIMEOUT_SECONDS, @@ -286,3 +326,17 @@ class TestStepsReachTheReport: already read, on every outcome including a setup error.""" for name in ("test_passes", "test_fails", "test_errors_in_setup"): assert tuple(prop for prop, _ in report[name])[:4] == ("package", "covers", "source", "step"), name + + +def test_a_failed_phase_s_own_report_carries_the_steps(tmp_path: Path) -> None: + """Plugins that read the failed setup or call report, not the teardown one + junitxml writes from, see where the test died too, oauth-live or not.""" + write_suite(tmp_path, {"test_consent.py": FAILED_PHASE_SUITE, "report_spy.py": REPORT_SPY_PLUGIN}) + child: Final = run_child_pytest(tmp_path, "-p", "report_spy", env={"E2E_MCP_OAUTH_LIVE": "1"}) + seen_path: Final = tmp_path / "failed-reports.jsonl" + assert seen_path.exists(), f"no failed report reached the spy:\n{child.stdout}\n{child.stderr}" + seen: Final = dict(map(FailedReport.validate_json, seen_path.read_text().splitlines())) + assert seen == { + "test_oauth_dies_on_consent": ("open the consent page",), + "test_plain_dies_on_consent": ("open the consent page",), + }, child.stdout diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 1f957a12574..1995909efba 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -337,7 +337,8 @@ def pytest_runtest_makereport( test most often dies (proxy not ready, key creation failing). The second attach replaces the first, so nothing is doubled. JUnit writes properties from the teardown report, which pytest builds from `item.user_properties` - after both of these have run. + after both of these have run. The setup and call reports carry them as well, + so a reader of a failed phase's own report sees where it died too. Teardown deliberately does not attach. Steps recorded by fixture finalizers are cleanup, and appending them would put "delete virtual key" after the step @@ -345,17 +346,17 @@ def pytest_runtest_makereport( finalizer that raises is still reported by JUnit with its own traceback. """ report = yield + if report.when in ("setup", "call"): + attach_step_properties(item) if item.get_closest_marker("mcp_oauth_live") is not None and call.excinfo is not None: # Publish code locations only, never exception messages, source text or locals. item.user_properties.append(("oauth_failure_phase", report.when)) item.user_properties.append(("oauth_exception_type", call.excinfo.type.__name__)) for entry in call.excinfo.traceback: item.user_properties.append(("oauth_frame", f"{Path(entry.path).name}:{entry.lineno + 1}:{entry.name}")) - report.user_properties = list(item.user_properties) if report.when == "call": item.stash[_CALL_PASSED] = report.passed - if report.when in ("setup", "call"): - attach_step_properties(item) + report.user_properties = list(item.user_properties) return report