From e373d0cb3b9d71d6e37303f441fa14b206584115 Mon Sep 17 00:00:00 2001 From: yassin Date: Fri, 17 Jul 2026 17:16:27 +0000 Subject: [PATCH] fix(tests/e2e): skip e2e tests on dead proxy; scope no-unit-tests rule to harness coverage Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- tests/e2e/CLAUDE.md | 2 +- tests/e2e/conftest.py | 32 ++++++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/tests/e2e/CLAUDE.md b/tests/e2e/CLAUDE.md index 0e1eafb5196..a916adc38d4 100644 --- a/tests/e2e/CLAUDE.md +++ b/tests/e2e/CLAUDE.md @@ -173,7 +173,7 @@ other... ``` ## Hard Rules -- no monkeypatching, mock tests or unit tests of any kind. if a contributor asks you to write an end to end test, do NOT stage a unit test with it. if you find a product gap, call it out in the PR description +- no monkeypatching or mock tests, and do not substitute unit tests for e2e feature coverage. if a contributor asks you to write an end to end test, do NOT stage a unit test alongside it; if you find a product gap, call it out in the PR description instead. tests that cover the harness itself (the coverage_registry collector, the claude_code drivers, and similar tooling under tests/e2e/) are allowed, carry no `e2e` marker, and run regardless of whether a proxy is up - use model management endpoints to create new models for a test. this could be in a conftest / inline for each test. ask the user what they want. diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 3aec104c861..51e7215c753 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -1,9 +1,11 @@ """Shared fixtures for all live e2e suites under tests/e2e/. -Design rule: hard failures only. Live tests (marked `e2e`) fail when no proxy -answers or when credentials/env are missing; they never skip. Pure unit coverage -of the harness itself carries no `e2e` marker and runs regardless of whether a -proxy is up. +Design rule: a test marked `e2e` skips when no proxy answers its liveness probe, +but once a request reaches the proxy any wrong behavior is a hard failure, never +a skip. The `logging/` suite is the deliberate exception: it hard-fails instead +of skipping when its proxy or credentials are missing. Pure coverage of the +harness itself carries no `e2e` marker and runs regardless of whether a proxy is +up. Lifecycle: the `resources` fixture maps the init -> run -> teardown contract (lifecycle.E2ECase) onto pytest - setup is init(), the test body is run(), and @@ -64,15 +66,29 @@ def _proxy_fail_reason() -> str | None: return None +_LOGGING_SUITE_DIR = Path(__file__).parent / "logging" + + +def _hard_fails_on_dead_proxy(item: pytest.Item) -> bool: + """The `logging/` suite deliberately hard-fails (never skips) when its proxy or + credentials are missing; every other suite skips when no proxy answers.""" + return _LOGGING_SUITE_DIR in item.path.parents + + def pytest_runtest_setup(item: pytest.Item) -> None: - """Hard-fail `e2e`-marked tests unless a proxy answers its liveness probe. - Unmarked tests (unit coverage of the harness) don't touch the proxy, so they - run even when none is up. Never skip for a missing proxy.""" + """Skip `e2e`-marked tests when no proxy answers its liveness probe, so an + absent proxy is a skip rather than a failure. Wrong behavior once a request + reaches the proxy is still a hard failure. The `logging/` suite hard-fails + instead of skipping. Unmarked tests (coverage of the harness) don't touch the + proxy, so they run even when none is up.""" if item.get_closest_marker("e2e") is None: return reason = _proxy_fail_reason() - if reason is not None: + if reason is None: + return + if _hard_fails_on_dead_proxy(item): pytest.fail(reason) + pytest.skip(reason) def pytest_runtest_call(item: pytest.Item) -> None: