From 298ba12709edb26a98506c7302b0c25efec4ab3f Mon Sep 17 00:00:00 2001 From: creatixchu Date: Fri, 25 Sep 2026 13:46:36 +0800 Subject: [PATCH] fix(cua): attempt all native fixture cleanup actions --- packages/experimental/cua/native/tests/e2e.py | 16 +++---- .../cua/native/tests/electron_e2e.py | 12 +++--- .../cua/native/tests/native_test_support.py | 9 ++++ .../cua/native/tests/test_cleanup.py | 42 +++++++++++++++++++ 4 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 packages/experimental/cua/native/tests/test_cleanup.py diff --git a/packages/experimental/cua/native/tests/e2e.py b/packages/experimental/cua/native/tests/e2e.py index 08e6acea27..cad3c536fd 100644 --- a/packages/experimental/cua/native/tests/e2e.py +++ b/packages/experimental/cua/native/tests/e2e.py @@ -14,7 +14,7 @@ import subprocess import tempfile import time -from native_test_support import ROOT, NativeHelper, line, stop +from native_test_support import ROOT, NativeHelper, line, run_cleanup, stop def check_file_panel(call, owner, primary_params, primary, directory, wait_state, button_target): @@ -241,13 +241,13 @@ def main(): helper.eof() print("PASS: capability preflight, two-window snapshots, scoped invalidation, AX readback, Unicode input, single scroll/right-click/drag dispatch, image dimensions, system file panel, stale app cleanup, EOF") finally: - if artifacts is not None: - (artifacts / "transcript.json").write_text(json.dumps(transcript, ensure_ascii=False, indent=2) + "\n") - if state_path.exists(): - (artifacts / "fixture-state.json").write_bytes(state_path.read_bytes()) - if helper is not None: - helper.stop() - stop(fixture) + def save_artifacts(): + if artifacts is not None: + (artifacts / "transcript.json").write_text(json.dumps(transcript, ensure_ascii=False, indent=2) + "\n") + if state_path.exists(): + (artifacts / "fixture-state.json").write_bytes(state_path.read_bytes()) + + run_cleanup(save_artifacts, *([helper.stop] if helper is not None else []), lambda: stop(fixture)) if __name__ == "__main__": diff --git a/packages/experimental/cua/native/tests/electron_e2e.py b/packages/experimental/cua/native/tests/electron_e2e.py index a3148ede5e..595a44e060 100644 --- a/packages/experimental/cua/native/tests/electron_e2e.py +++ b/packages/experimental/cua/native/tests/electron_e2e.py @@ -13,7 +13,7 @@ import subprocess import tempfile import time -from native_test_support import ROOT, NativeHelper, line, stop_group +from native_test_support import ROOT, NativeHelper, line, run_cleanup, stop_group def main(): @@ -128,11 +128,11 @@ def main(): print(diagnostics.read()[-8192:]) raise finally: - if args.artifacts: - (args.artifacts / 'transcript.json').write_text(json.dumps(transcript, ensure_ascii=False, indent=2) + '\n') - if helper is not None: - helper.stop() - stop_group(fixture) + def save_artifacts(): + if args.artifacts: + (args.artifacts / 'transcript.json').write_text(json.dumps(transcript, ensure_ascii=False, indent=2) + '\n') + + run_cleanup(save_artifacts, *([helper.stop] if helper is not None else []), lambda: stop_group(fixture)) if __name__ == '__main__': diff --git a/packages/experimental/cua/native/tests/native_test_support.py b/packages/experimental/cua/native/tests/native_test_support.py index 86351c5601..61d1b6aa40 100644 --- a/packages/experimental/cua/native/tests/native_test_support.py +++ b/packages/experimental/cua/native/tests/native_test_support.py @@ -1,16 +1,25 @@ """Owned JSONL helper processes shared by opt-in macOS application checks.""" import json +from contextlib import ExitStack import os from pathlib import Path import select import signal import subprocess import time +from typing import Callable ROOT = Path(__file__).resolve().parents[1] BINARY = ROOT / ".build" / "release" / "dsh-cua-native" +def run_cleanup(*actions: Callable[[], None]) -> None: + """Attempt every cleanup in order and preserve chained cleanup failures.""" + with ExitStack() as pending: + for action in reversed(actions): + pending.callback(action) + + def line(process, timeout=15): ready, _, _ = select.select([process.stdout], [], [], timeout) if not ready: diff --git a/packages/experimental/cua/native/tests/test_cleanup.py b/packages/experimental/cua/native/tests/test_cleanup.py new file mode 100644 index 0000000000..e83e959591 --- /dev/null +++ b/packages/experimental/cua/native/tests/test_cleanup.py @@ -0,0 +1,42 @@ +"""Fixture artifacts and every owned process receive their cleanup attempt.""" +import unittest + +from native_test_support import run_cleanup + + +class CleanupTests(unittest.TestCase): + def test_artifact_failure_does_not_skip_either_process(self): + calls = [] + + def artifacts(): + calls.append("artifacts") + raise OSError("cannot save artifacts") + + with self.assertRaisesRegex(OSError, "cannot save artifacts"): + run_cleanup(artifacts, lambda: calls.append("helper"), lambda: calls.append("fixture")) + self.assertEqual(calls, ["artifacts", "helper", "fixture"]) + + def test_helper_failure_still_stops_the_fixture_and_keeps_the_first_error(self): + calls = [] + + def artifacts(): + calls.append("artifacts") + raise OSError("cannot save artifacts") + + def helper(): + calls.append("helper") + raise RuntimeError("helper did not exit") + + with self.assertRaisesRegex(RuntimeError, "helper did not exit") as caught: + run_cleanup(artifacts, helper, lambda: calls.append("fixture")) + self.assertEqual(calls, ["artifacts", "helper", "fixture"]) + self.assertIsInstance(caught.exception.__context__, OSError) + + def test_successful_cleanup_runs_in_order(self): + calls = [] + run_cleanup(lambda: calls.append("helper"), lambda: calls.append("fixture")) + self.assertEqual(calls, ["helper", "fixture"]) + + +if __name__ == "__main__": + unittest.main()