From 3b8b9df4bf99713d93b786264dcc801400905ab5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 26 Sep 2026 20:41:59 +0000 Subject: [PATCH 1/2] fix: retain crash ids and pointer-capture diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A worker that dies before writing a report now adds the heartbeat or log test id to run-level failed_ids, so last-failed can rerun it. Cancelled fail-fast siblings stay out of that set. Diagnostics finalize after pointer capture; a pointer-only failure keeps a privacy-safe snapshot. Co-authored-by: Nikola Perović --- tests/e2e/run.py | 148 ++++++++++++++-------- tests/e2e/runner_selfcheck_cases.py | 23 ++++ tests/test_e2e_policy.py | 184 ++++++++++++++++++++++++++++ tests/test_e2e_sharding.py | 69 ++++++++++- 4 files changed, 374 insertions(+), 50 deletions(-) diff --git a/tests/e2e/run.py b/tests/e2e/run.py index e2dd2a51..a2646847 100644 --- a/tests/e2e/run.py +++ b/tests/e2e/run.py @@ -488,6 +488,80 @@ def _format_watchdog_detail(hb: dict, threshold_s: int, age_s: float) -> str: ) +def _finalize_failure_diagnostics( + *, + ok: bool, + failed_ids, + tier: str, + note: str, + persist_history: bool, + pointer_returncode, +) -> None: + """Write or clear ``.tests/e2e-failure-diagnostics.json`` after the run. + + Called only after selected E2E tests and, when enabled, pointer capture. + A green suite clears a stale snapshot. Pointer-capture failure persists a + privacy-safe record (no screenshots, traces, or library content) instead + of clearing. E2E failures keep the existing test snapshot; pointer capture + does not run in that case. + """ + pointer_failed = pointer_returncode not in (None, 0) + if not ok and persist_history: + # Watchdog / parallel paths already wrote; refresh last-failed attach + # and fill serial assertion gaps without inventing a prior-run kind. + existing = load_failure_diagnostics(REPO / FAILURE_DIAGNOSTICS_PATH) or {} + hb = get_e2e_heartbeat() + stuck = "" + if failed_ids: + stuck = failed_ids[0] + elif hb.get("test_id"): + stuck = str(hb.get("test_id") or "") + if existing.get("kind"): + payload = dict(existing) + payload["failed_ids"] = list(failed_ids) or list( + existing.get("failed_ids") or [] + ) + if stuck and not payload.get("stuck_test_id"): + payload["stuck_test_id"] = stuck + payload["tier"] = tier + payload["note"] = note + _persist_failure_diagnostics(payload) + else: + _persist_failure_diagnostics( + { + "kind": "failure", + "failed_ids": list(failed_ids), + "stuck_test_id": stuck, + "last_stage": str(hb.get("stage") or ""), + "watchdog": False, + "detail": "", + "tier": tier, + "note": note, + } + ) + return + if ok and pointer_failed: + # Exact shape: ids/stages/return code only. Do not attach traces, + # screenshots, or last-failed library context. + try: + save_failure_diagnostics( + REPO / FAILURE_DIAGNOSTICS_PATH, + { + "kind": "pointer-capture", + "failed_ids": [], + "watchdog": False, + "pointer_capture_returncode": pointer_returncode, + }, + ) + except Exception: + pass + return + if ok: + # Success, including --no-pointer-capture and benchmark passes, must + # not leave a prior failure snapshot for this run. + clear_failure_diagnostics(REPO / FAILURE_DIAGNOSTICS_PATH) + + def _persist_failure_diagnostics(payload: dict) -> None: """Best-effort machine-readable failure snapshot under ``.tests/``. @@ -1051,22 +1125,28 @@ def _shutdown(signum, _frame): if not ok: _print_worker_failure(worker, jobs, report) tid, stage = _hang_attribution(worker) + reported_failed = list((report or {}).get("failed_ids") or []) if report is not None: stage = str(report.get("stage") or stage or "") - if not tid: - ids = report.get("failed_ids") or [] - tid = ids[0] if ids else tid + if not tid and reported_failed: + tid = reported_failed[0] + # A crash before the report leaves failed_ids empty even + # when heartbeat/log still names the active test. Record + # that id so last-failed can rerun it. Reported ids were + # copied above. Cancelled --fail-fast siblings are marked + # cancelled and never enter this branch. + if not reported_failed and tid and tid not in failed_ids: + failed_ids.append(tid) is_watchdog = bool((report or {}).get("watchdog")) if report is None or is_watchdog: kind = "watchdog" if is_watchdog else "worker-crash" record_failed = list( - (report or {}).get("failed_ids") - or ([tid] if tid else []) + reported_failed or ([tid] if tid else []) ) - elif report.get("failed_ids"): + elif reported_failed: kind = "assertion-failure" - record_failed = list(report.get("failed_ids") or []) - tid = record_failed[0] if record_failed else tid + record_failed = list(reported_failed) + tid = record_failed[0] stage = "" else: kind = "worker-crash" @@ -1995,47 +2075,6 @@ def _env_true(name: str) -> bool: if previous_ids: print("Cleared last-failed (all previously failed tests resolved)") - # Machine-readable hang/failure snapshot for CI artifacts. - # Watchdog / parallel paths already wrote; refresh last-failed attach - # and fill serial assertion gaps without inventing a prior-run kind. - if not ok: - existing = load_failure_diagnostics(REPO / FAILURE_DIAGNOSTICS_PATH) or {} - hb = get_e2e_heartbeat() - stuck = "" - if failed_ids: - stuck = failed_ids[0] - elif hb.get("test_id"): - stuck = str(hb.get("test_id") or "") - if existing.get("kind"): - payload = dict(existing) - payload["failed_ids"] = list(failed_ids) or list( - existing.get("failed_ids") or [] - ) - if stuck and not payload.get("stuck_test_id"): - payload["stuck_test_id"] = stuck - payload["tier"] = tier - payload["note"] = note - _persist_failure_diagnostics(payload) - else: - _persist_failure_diagnostics( - { - "kind": "failure", - "failed_ids": list(failed_ids), - "stuck_test_id": stuck, - "last_stage": str(hb.get("stage") or ""), - "watchdog": False, - "detail": "", - "tier": tier, - "note": note, - } - ) - else: - clear_failure_diagnostics(REPO / FAILURE_DIAGNOSTICS_PATH) - elif ok: - # Benchmark modes skip history writes but must not leave a prior - # failure snapshot pretending to belong to this run. - clear_failure_diagnostics(REPO / FAILURE_DIAGNOSTICS_PATH) - pointer = None if ok and not args.no_pointer_capture: # Once per run, in the parent, after every shard has passed -- never @@ -2047,6 +2086,17 @@ def _env_true(name: str) -> bool: elif not ok and not args.no_pointer_capture: print("skipping pointer_capture.py because E2E tests failed", file=sys.stderr) + # Finalize after both the selected tests and pointer capture. Clearing on + # a green suite before pointer capture dropped a later pointer failure. + _finalize_failure_diagnostics( + ok=ok, + failed_ids=failed_ids, + tier=tier, + note=note, + persist_history=persist_history, + pointer_returncode=pointer, + ) + code = run_exit_code(ok, pointer) if code == 0: print( diff --git a/tests/e2e/runner_selfcheck_cases.py b/tests/e2e/runner_selfcheck_cases.py index 02549ae4..ec186906 100644 --- a/tests/e2e/runner_selfcheck_cases.py +++ b/tests/e2e/runner_selfcheck_cases.py @@ -41,3 +41,26 @@ def test_never_finishes(self): # Exercises the per-test watchdog under a short PRKS_E2E_TEST_WATCHDOG. # Must not be collected by ordinary discovery (this module is not test_*). time.sleep(3600) + + +class FailFastSiblingCases(unittest.TestCase): + """One worker fails only after its sibling has an active test id. + + Used to prove --fail-fast cancellation does not record the stopped sibling + as a failed test. Not collected by ordinary discovery. + """ + + def test_runs_until_cancelled(self): + sentinel = os.environ.get("PRKS_E2E_CANCEL_SENTINEL") + if sentinel: + with open(sentinel, "w", encoding="utf-8") as handle: + handle.write("started") + time.sleep(30) + + def test_fails_once_sibling_is_active(self): + sentinel = os.environ.get("PRKS_E2E_CANCEL_SENTINEL") + if sentinel: + deadline = time.time() + 15 + while time.time() < deadline and not os.path.exists(sentinel): + time.sleep(0.05) + self.assertEqual("expected", "actual") diff --git a/tests/test_e2e_policy.py b/tests/test_e2e_policy.py index e7230c37..472afdd6 100644 --- a/tests/test_e2e_policy.py +++ b/tests/test_e2e_policy.py @@ -2,6 +2,7 @@ from __future__ import annotations import contextlib +import io import json import os import shutil @@ -2092,5 +2093,188 @@ def test_no_sigalrm_watchdog_in_runner(self): self.assertFalse(hasattr(runner, "FullGateTimeoutError")) +class CrashLastFailedRetentionTests(unittest.TestCase): + def test_crash_attributed_id_is_retained_in_last_failed(self): + """main() persists the active id when a worker dies without a report.""" + from tests.e2e import run as runner + + crash_id = ( + "tests.e2e.runner_selfcheck_cases.CrashingCases.test_kills_the_worker" + ) + pass_id = "tests.e2e.runner_selfcheck_cases.PassingCases.test_first" + previous = os.environ.get("PRKS_E2E") + os.environ["PRKS_E2E"] = "1" + try: + with tempfile.TemporaryDirectory(prefix="prks-crash-last-") as raw: + root = Path(raw) + last_path = root / "e2e-last-failed.json" + diag_path = root / "e2e-failure-diagnostics.json" + with mock.patch.object(runner, "LAST_FAILED_PATH", last_path): + with mock.patch.object( + runner, "FAILURE_DIAGNOSTICS_PATH", diag_path + ): + with mock.patch.object( + runner, + "discover_test_ids", + return_value=[pass_id, crash_id], + ): + with mock.patch.object(runner, "ensure_chromium_installed"): + with mock.patch.object(runner, "_persist_timings"): + with mock.patch.object(runner, "_print_slowest"): + with contextlib.redirect_stdout(io.StringIO()): + with contextlib.redirect_stderr(io.StringIO()): + code = runner.main( + [ + pass_id, + crash_id, + "--jobs", + "2", + "--no-pointer-capture", + ] + ) + self.assertEqual(code, 1) + data = policy.load_last_failed(last_path) + self.assertIsNotNone(data) + self.assertEqual(data["test_ids"], [crash_id]) + loaded = policy.load_failure_diagnostics(diag_path) + self.assertIsNotNone(loaded) + self.assertIn(crash_id, loaded["failed_ids"]) + self.assertNotEqual(loaded.get("kind"), "pointer-capture") + finally: + if previous is None: + os.environ.pop("PRKS_E2E", None) + else: + os.environ["PRKS_E2E"] = previous + + +class PointerCaptureDiagnosticsTests(unittest.TestCase): + """Diagnostics finalize after pointer capture, without traces or content.""" + + def _invoke( + self, + *, + tests_ok, + failed_ids=None, + pointer_code=0, + no_pointer=False, + during_tests=None, + during_pointer=None, + ): + from tests.e2e import run as runner + + test_id = "tests.e2e.fake.DiagTests.test_one" + failed = list(failed_ids or []) + observed = {test_id: 0.2} if tests_ok else {} + + def run_serial(*_args, **_kwargs): + if during_tests is not None: + during_tests(diag) + return (tests_ok, observed, failed, {}) + + def run_pointer(): + if during_pointer is not None: + during_pointer(diag) + return pointer_code + + with tempfile.TemporaryDirectory(prefix="prks-ptr-diag-") as raw: + repo = Path(raw) + diag = repo / ".tests" / "e2e-failure-diagnostics.json" + argv = [test_id, "--jobs", "1"] + if no_pointer: + argv.append("--no-pointer-capture") + with contextlib.ExitStack() as stack: + enter = stack.enter_context + enter(mock.patch.object(runner, "REPO", repo)) + enter(mock.patch.object(runner, "ensure_chromium_installed")) + enter( + mock.patch.object( + runner, "discover_test_ids", return_value=[test_id] + ) + ) + enter(mock.patch.object(runner, "run_serial", side_effect=run_serial)) + pointer = enter( + mock.patch.object( + runner, "_run_pointer_capture", side_effect=run_pointer + ) + ) + enter(mock.patch.object(runner, "_print_slowest")) + enter(mock.patch.object(runner, "_persist_timings")) + enter(contextlib.redirect_stdout(io.StringIO())) + enter(contextlib.redirect_stderr(io.StringIO())) + code = runner.main(argv) + loaded = policy.load_failure_diagnostics(diag) + return code, loaded, pointer + + def test_pass_and_pointer_pass_clears_diagnostics(self): + def plant(diag): + policy.save_failure_diagnostics( + diag, {"kind": "stale", "failed_ids": ["tests.e2e.fake.T.test_old"]} + ) + + code, loaded, pointer = self._invoke( + tests_ok=True, pointer_code=0, during_pointer=plant + ) + self.assertEqual(code, 0) + pointer.assert_called_once() + self.assertIsNone(loaded) + + def test_e2e_failure_keeps_existing_diagnostics(self): + test_id = "tests.e2e.fake.DiagTests.test_one" + + def plant(diag): + policy.save_failure_diagnostics( + diag, + { + "kind": "worker-crash", + "failed_ids": [test_id], + "stuck_test_id": test_id, + "last_stage": "APP_READY", + "watchdog": False, + }, + ) + + code, loaded, pointer = self._invoke( + tests_ok=False, + failed_ids=[test_id], + during_tests=plant, + ) + self.assertEqual(code, 1) + pointer.assert_not_called() + self.assertIsNotNone(loaded) + self.assertEqual(loaded["kind"], "worker-crash") + self.assertEqual(loaded["failed_ids"], [test_id]) + self.assertEqual(loaded["last_stage"], "APP_READY") + self.assertNotIn("pointer_capture_returncode", loaded) + self.assertNotIn("screenshot", loaded) + self.assertNotIn("trace", loaded) + + def test_pointer_failure_writes_privacy_safe_diagnostics(self): + code, loaded, pointer = self._invoke(tests_ok=True, pointer_code=7) + self.assertEqual(code, 1) + pointer.assert_called_once() + self.assertEqual( + loaded, + { + "kind": "pointer-capture", + "failed_ids": [], + "watchdog": False, + "pointer_capture_returncode": 7, + }, + ) + + def test_no_pointer_capture_success_clears_diagnostics(self): + def plant(diag): + policy.save_failure_diagnostics( + diag, {"kind": "stale", "failed_ids": ["tests.e2e.fake.T.test_old"]} + ) + + code, loaded, pointer = self._invoke( + tests_ok=True, no_pointer=True, during_tests=plant + ) + self.assertEqual(code, 0) + pointer.assert_not_called() + self.assertIsNone(loaded) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_e2e_sharding.py b/tests/test_e2e_sharding.py index 7d934832..62e0a108 100644 --- a/tests/test_e2e_sharding.py +++ b/tests/test_e2e_sharding.py @@ -811,7 +811,8 @@ def test_failing_worker_records_failed_ids(self): with contextlib.redirect_stdout(sink), contextlib.redirect_stderr(sink): ok, _observed, failed, _phases = runner.run_parallel(ids, 2, {}, False) self.assertFalse(ok) - self.assertTrue(any(fid.endswith("test_fails") for fid in failed)) + self.assertEqual(len(failed), 1) + self.assertTrue(failed[0].endswith("FailingCases.test_fails")) def test_a_failing_worker_fails_the_parent(self): ids = _ids(_CASES, "PassingCases", "test_first") + _ids( @@ -827,6 +828,72 @@ def test_a_crashed_worker_fails_the_parent(self): ok, _ = self._run(ids, 2) self.assertFalse(ok) + def test_crash_without_report_keeps_active_test_in_failed_ids(self): + """Heartbeat/log attribution must land in run-level failed_ids.""" + crash_id = _ids(_CASES, "CrashingCases", "test_kills_the_worker")[0] + pass_id = _ids(_CASES, "PassingCases", "test_first")[0] + sink = io.StringIO() + with tempfile.TemporaryDirectory(prefix="prks-crash-attr-") as raw: + diag = Path(raw) / "e2e-failure-diagnostics.json" + with _import_runner() as runner: + with mock.patch.object(runner, "FAILURE_DIAGNOSTICS_PATH", diag): + with mock.patch.object( + runner, "LAST_FAILED_PATH", Path(raw) / "absent-last-failed.json" + ): + with contextlib.redirect_stdout(sink), contextlib.redirect_stderr( + sink + ): + ok, observed, failed, _phases = runner.run_parallel( + [pass_id, crash_id], 2, {}, False + ) + self.assertFalse(ok) + self.assertNotIn(crash_id, observed) + self.assertEqual(failed, [crash_id]) + loaded = json.loads(diag.read_text(encoding="utf-8")) + self.assertIn(crash_id, loaded["failed_ids"]) + self.assertEqual(loaded["failed_ids"].count(crash_id), 1) + self.assertIn(crash_id, sink.getvalue()) + + def test_fail_fast_cancelled_sibling_is_not_recorded_as_failed(self): + """Stopping a peer for --fail-fast must not add its active test id.""" + fail_id = _ids( + _CASES, "FailFastSiblingCases", "test_fails_once_sibling_is_active" + )[0] + cancel_id = _ids( + _CASES, "FailFastSiblingCases", "test_runs_until_cancelled" + )[0] + sink = io.StringIO() + previous = os.environ.get("PRKS_E2E_CANCEL_SENTINEL") + with tempfile.TemporaryDirectory(prefix="prks-ff-cancel-") as raw: + sentinel = str(Path(raw) / "sibling-started") + os.environ["PRKS_E2E_CANCEL_SENTINEL"] = sentinel + try: + with _import_runner() as runner: + with mock.patch.object( + runner, "FAILURE_DIAGNOSTICS_PATH", Path(raw) / "diag.json" + ): + with mock.patch.object( + runner, + "LAST_FAILED_PATH", + Path(raw) / "absent-last-failed.json", + ): + with contextlib.redirect_stdout( + sink + ), contextlib.redirect_stderr(sink): + ok, _observed, failed, _phases = runner.run_parallel( + [fail_id, cancel_id], 2, {}, True + ) + finally: + if previous is None: + os.environ.pop("PRKS_E2E_CANCEL_SENTINEL", None) + else: + os.environ["PRKS_E2E_CANCEL_SENTINEL"] = previous + self.assertTrue(os.path.isfile(sentinel)) + self.assertFalse(ok) + self.assertEqual(failed, [fail_id]) + self.assertNotIn(cancel_id, failed) + self.assertIn("fail-fast", sink.getvalue()) + def test_per_test_watchdog_kills_hanging_worker_and_names_stage(self): """Short watchdog must terminate a sleeping selfcheck without retry.""" hang_id = _ids(_CASES, "HangingCases", "test_never_finishes")[0] From faa09eaf46f2dbdb95d14bd7dbce57fe305cceaf Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 26 Sep 2026 20:58:14 +0000 Subject: [PATCH 2/2] fix: keep passed tests out of last-failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Failure ids now come only from the heartbeat's current test id. The worker log stays diagnostic text, so a finished test that dies before its report is not rerun. The CI pointer job writes and uploads the same privacy-safe snapshot the local finalizer uses. Co-authored-by: Nikola Perović --- .github/workflows/e2e-gate.yml | 22 +++++++- tests/e2e/run.py | 87 ++++++++++++++++++++--------- tests/e2e/runner_selfcheck_cases.py | 22 +++++++- tests/test_e2e_policy.py | 87 +++++++++++++++++++++++++++++ tests/test_e2e_sharding.py | 32 ++++++++++- 5 files changed, 220 insertions(+), 30 deletions(-) diff --git a/.github/workflows/e2e-gate.yml b/.github/workflows/e2e-gate.yml index 04fde31f..e3b34bfd 100644 --- a/.github/workflows/e2e-gate.yml +++ b/.github/workflows/e2e-gate.yml @@ -277,7 +277,27 @@ jobs: - name: Run pointer_capture once for the gate env: PRKS_E2E: "1" - run: python tests/browser/pointer_capture.py + run: | + set +e + python tests/browser/pointer_capture.py + code=$? + set -e + if [[ "$code" -ne 0 ]]; then + # This job does not go through tests/e2e/run.py main(), so record + # the same privacy-safe snapshot the local finalizer writes. + POINTER_RC="$code" python -c 'import os; from tests.e2e.run import persist_pointer_capture_failure; persist_pointer_capture_failure(int(os.environ["POINTER_RC"]))' + exit "$code" + fi + + - name: Upload pointer failure diagnostics + if: failure() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: e2e-pointer-failure-diagnostics + path: .tests/e2e-failure-diagnostics.json + include-hidden-files: true + if-no-files-found: error + retention-days: 7 e2e-result: permissions: diff --git a/tests/e2e/run.py b/tests/e2e/run.py index a2646847..45a8998a 100644 --- a/tests/e2e/run.py +++ b/tests/e2e/run.py @@ -541,25 +541,35 @@ def _finalize_failure_diagnostics( ) return if ok and pointer_failed: - # Exact shape: ids/stages/return code only. Do not attach traces, - # screenshots, or last-failed library context. - try: + persist_pointer_capture_failure(pointer_returncode) + return + if ok: + # Success, including --no-pointer-capture and benchmark passes, must + # not leave a prior failure snapshot for this run. + clear_failure_diagnostics(REPO / FAILURE_DIAGNOSTICS_PATH) + + +def persist_pointer_capture_failure(returncode) -> bool: + """Write the privacy-safe pointer-capture snapshot and report success. + + Same payload the local runner stores when tests pass and pointer capture + fails. No screenshots, traces, or library content. CI's pointer job calls + this directly because that job does not go through ``main()``. + """ + try: + return bool( save_failure_diagnostics( REPO / FAILURE_DIAGNOSTICS_PATH, { "kind": "pointer-capture", "failed_ids": [], "watchdog": False, - "pointer_capture_returncode": pointer_returncode, + "pointer_capture_returncode": returncode, }, ) - except Exception: - pass - return - if ok: - # Success, including --no-pointer-capture and benchmark passes, must - # not leave a prior failure snapshot for this run. - clear_failure_diagnostics(REPO / FAILURE_DIAGNOSTICS_PATH) + ) + except Exception: + return False def _persist_failure_diagnostics(payload: dict) -> None: @@ -954,8 +964,26 @@ def _last_diag_stage(log_file) -> tuple[str, str]: return stage, tid +def _current_heartbeat_test_id(worker) -> str: + """In-flight unittest id from the heartbeat file, or empty. + + ``stopTest`` and ``clear_e2e_heartbeat`` drop ``test_id`` before the + report is written. An empty id means nothing is currently running — the + worker log is not a substitute, because its last line may be a test that + already passed. + """ + hb = _read_heartbeat(worker.get("heartbeat_file")) + if not hb: + return "" + return str(hb.get("test_id") or "").strip() + + def _hang_attribution(worker) -> tuple[str, str]: - """Best-effort (test_id, stage) for a hung or watchdog-killed worker.""" + """Best-effort (test_id, stage) for diagnostic text. + + Prefer the heartbeat. The log / diag-line fallback names whatever printed + last so a human can see it; it must not be stored as a failed test id. + """ hb = _read_heartbeat(worker.get("heartbeat_file")) if hb and (hb.get("test_id") or hb.get("stage")): return str(hb.get("test_id") or ""), str(hb.get("stage") or "") @@ -1107,9 +1135,9 @@ def _shutdown(signum, _frame): if fid not in failed_ids: failed_ids.append(fid) if report.get("watchdog") and not report.get("failed_ids"): - tid, _stage = _hang_attribution(worker) - if tid and tid not in failed_ids: - failed_ids.append(tid) + active_id = _current_heartbeat_test_id(worker) + if active_id and active_id not in failed_ids: + failed_ids.append(active_id) ok = report is not None and rc == 0 print( "[E2E %d/%d] %s — %.1fs (%d tests)" @@ -1124,25 +1152,29 @@ def _shutdown(signum, _frame): sys.stdout.flush() if not ok: _print_worker_failure(worker, jobs, report) - tid, stage = _hang_attribution(worker) + _, stage = _hang_attribution(worker) + active_id = _current_heartbeat_test_id(worker) reported_failed = list((report or {}).get("failed_ids") or []) if report is not None: stage = str(report.get("stage") or stage or "") - if not tid and reported_failed: - tid = reported_failed[0] - # A crash before the report leaves failed_ids empty even - # when heartbeat/log still names the active test. Record - # that id so last-failed can rerun it. Reported ids were - # copied above. Cancelled --fail-fast siblings are marked - # cancelled and never enter this branch. - if not reported_failed and tid and tid not in failed_ids: - failed_ids.append(tid) + # Rerun only a test the heartbeat still marks in flight. + # A cleared heartbeat (stopTest / pre-report clear) must + # not promote the last log line — that test may have + # passed. Log attribution stays in the printed diagnostic. + # Cancelled --fail-fast siblings never enter this branch. + if ( + not reported_failed + and active_id + and active_id not in failed_ids + ): + failed_ids.append(active_id) is_watchdog = bool((report or {}).get("watchdog")) if report is None or is_watchdog: kind = "watchdog" if is_watchdog else "worker-crash" record_failed = list( - reported_failed or ([tid] if tid else []) + reported_failed or ([active_id] if active_id else []) ) + tid = active_id or (reported_failed[0] if reported_failed else "") elif reported_failed: kind = "assertion-failure" record_failed = list(reported_failed) @@ -1150,7 +1182,8 @@ def _shutdown(signum, _frame): stage = "" else: kind = "worker-crash" - record_failed = [tid] if tid else [] + record_failed = [active_id] if active_id else [] + tid = active_id or "" worker_failure_records.append( { "kind": kind, diff --git a/tests/e2e/runner_selfcheck_cases.py b/tests/e2e/runner_selfcheck_cases.py index ec186906..1e8b1810 100644 --- a/tests/e2e/runner_selfcheck_cases.py +++ b/tests/e2e/runner_selfcheck_cases.py @@ -43,6 +43,19 @@ def test_never_finishes(self): time.sleep(3600) +class ClearedHeartbeatCrashCases(unittest.TestCase): + def test_passes_then_dies_after_heartbeat_clear(self): + """Finish the test's heartbeat, then die before the worker report. + + Mirrors run_worker clearing the heartbeat in ``finally`` and exiting + before the report file exists. The log still names this test. + """ + from tests.e2e.harness import clear_e2e_heartbeat + + clear_e2e_heartbeat() + os._exit(3) + + class FailFastSiblingCases(unittest.TestCase): """One worker fails only after its sibling has an active test id. @@ -50,12 +63,19 @@ class FailFastSiblingCases(unittest.TestCase): as a failed test. Not collected by ordinary discovery. """ + # Parent fail-fast SIGTERMs this worker. The loop is the wait; surviving + # it means cancellation never arrived. + _CANCEL_WAIT_S = 8.0 + def test_runs_until_cancelled(self): sentinel = os.environ.get("PRKS_E2E_CANCEL_SENTINEL") if sentinel: with open(sentinel, "w", encoding="utf-8") as handle: handle.write("started") - time.sleep(30) + deadline = time.monotonic() + self._CANCEL_WAIT_S + while time.monotonic() < deadline: + time.sleep(0.05) + self.fail("parent did not cancel this worker") def test_fails_once_sibling_is_active(self): sentinel = os.environ.get("PRKS_E2E_CANCEL_SENTINEL") diff --git a/tests/test_e2e_policy.py b/tests/test_e2e_policy.py index 472afdd6..0cd746d3 100644 --- a/tests/test_e2e_policy.py +++ b/tests/test_e2e_policy.py @@ -2146,6 +2146,58 @@ def test_crash_attributed_id_is_retained_in_last_failed(self): else: os.environ["PRKS_E2E"] = previous + def test_cleared_heartbeat_is_not_retained_in_last_failed(self): + """A finished test that dies before the report must not be rerun.""" + from tests.e2e import run as runner + + finished_id = ( + "tests.e2e.runner_selfcheck_cases.ClearedHeartbeatCrashCases." + "test_passes_then_dies_after_heartbeat_clear" + ) + pass_id = "tests.e2e.runner_selfcheck_cases.PassingCases.test_first" + previous = os.environ.get("PRKS_E2E") + os.environ["PRKS_E2E"] = "1" + try: + with tempfile.TemporaryDirectory(prefix="prks-cleared-last-") as raw: + root = Path(raw) + last_path = root / "e2e-last-failed.json" + diag_path = root / "e2e-failure-diagnostics.json" + with mock.patch.object(runner, "LAST_FAILED_PATH", last_path): + with mock.patch.object( + runner, "FAILURE_DIAGNOSTICS_PATH", diag_path + ): + with mock.patch.object( + runner, + "discover_test_ids", + return_value=[pass_id, finished_id], + ): + with mock.patch.object(runner, "ensure_chromium_installed"): + with mock.patch.object(runner, "_persist_timings"): + with mock.patch.object(runner, "_print_slowest"): + with contextlib.redirect_stdout(io.StringIO()): + with contextlib.redirect_stderr(io.StringIO()): + code = runner.main( + [ + pass_id, + finished_id, + "--jobs", + "2", + "--no-pointer-capture", + ] + ) + self.assertEqual(code, 1) + data = policy.load_last_failed(last_path) + retained = (data or {}).get("test_ids") or [] + self.assertNotIn(finished_id, retained) + loaded = policy.load_failure_diagnostics(diag_path) + self.assertIsNotNone(loaded) + self.assertNotIn(finished_id, loaded.get("failed_ids") or []) + finally: + if previous is None: + os.environ.pop("PRKS_E2E", None) + else: + os.environ["PRKS_E2E"] = previous + class PointerCaptureDiagnosticsTests(unittest.TestCase): """Diagnostics finalize after pointer capture, without traces or content.""" @@ -2276,5 +2328,40 @@ def plant(diag): self.assertIsNone(loaded) +class PointerCaptureCiArtifactTests(unittest.TestCase): + def test_persist_pointer_capture_failure_writes_exact_snapshot(self): + from tests.e2e import run as runner + + with tempfile.TemporaryDirectory(prefix="prks-ptr-fn-") as raw: + root = Path(raw) + with mock.patch.object(runner, "REPO", root): + self.assertTrue(runner.persist_pointer_capture_failure(9)) + loaded = policy.load_failure_diagnostics( + root / ".tests" / "e2e-failure-diagnostics.json" + ) + self.assertEqual( + loaded, + { + "kind": "pointer-capture", + "failed_ids": [], + "watchdog": False, + "pointer_capture_returncode": 9, + }, + ) + + def test_pointer_job_writes_and_uploads_failure_diagnostics(self): + text = Path(".github/workflows/e2e-gate.yml").read_text(encoding="utf-8") + start = text.index(" e2e-pointer:") + end = text.index("\n e2e-result:") + job = text[start:end] + self.assertIn("python tests/browser/pointer_capture.py", job) + self.assertIn("persist_pointer_capture_failure", job) + self.assertIn("e2e-failure-diagnostics.json", job) + self.assertIn("include-hidden-files: true", job) + self.assertIn("Upload pointer failure diagnostics", job) + self.assertNotIn("screenshot", job.lower()) + self.assertNotIn("trace", job.lower()) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_e2e_sharding.py b/tests/test_e2e_sharding.py index 62e0a108..54f4a8f3 100644 --- a/tests/test_e2e_sharding.py +++ b/tests/test_e2e_sharding.py @@ -829,7 +829,7 @@ def test_a_crashed_worker_fails_the_parent(self): self.assertFalse(ok) def test_crash_without_report_keeps_active_test_in_failed_ids(self): - """Heartbeat/log attribution must land in run-level failed_ids.""" + """An in-flight heartbeat id is a failed id when the worker dies.""" crash_id = _ids(_CASES, "CrashingCases", "test_kills_the_worker")[0] pass_id = _ids(_CASES, "PassingCases", "test_first")[0] sink = io.StringIO() @@ -854,6 +854,36 @@ def test_crash_without_report_keeps_active_test_in_failed_ids(self): self.assertEqual(loaded["failed_ids"].count(crash_id), 1) self.assertIn(crash_id, sink.getvalue()) + def test_cleared_heartbeat_does_not_record_finished_test_as_failed(self): + """A passed test whose heartbeat was cleared is diagnostic text only.""" + finished_id = _ids( + _CASES, + "ClearedHeartbeatCrashCases", + "test_passes_then_dies_after_heartbeat_clear", + )[0] + pass_id = _ids(_CASES, "PassingCases", "test_first")[0] + sink = io.StringIO() + with tempfile.TemporaryDirectory(prefix="prks-cleared-hb-") as raw: + diag = Path(raw) / "e2e-failure-diagnostics.json" + with _import_runner() as runner: + with mock.patch.object(runner, "FAILURE_DIAGNOSTICS_PATH", diag): + with mock.patch.object( + runner, "LAST_FAILED_PATH", Path(raw) / "absent-last-failed.json" + ): + with contextlib.redirect_stdout(sink), contextlib.redirect_stderr( + sink + ): + ok, _observed, failed, _phases = runner.run_parallel( + [pass_id, finished_id], 2, {}, False + ) + loaded = json.loads(diag.read_text(encoding="utf-8")) + self.assertFalse(ok) + self.assertNotIn(finished_id, failed) + self.assertNotIn(finished_id, loaded.get("failed_ids") or []) + for record in loaded.get("workers") or []: + self.assertNotIn(finished_id, record.get("failed_ids") or []) + self.assertIn("last started test: %s" % finished_id, sink.getvalue()) + def test_fail_fast_cancelled_sibling_is_not_recorded_as_failed(self): """Stopping a peer for --fail-fast must not add its active test id.""" fail_id = _ids(