From e663dec79e21b46d6fba7f6e4ef9bf9899ad925b Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 23 Sep 2026 09:29:40 -0400 Subject: [PATCH] [trace_replay] (P7): the device runner starts from a POSIX host and a dead repeat archives no stale picture --- .../trace_replay/run_android_retrace_local.py | 49 +++++++- tools/trace_replay/test_compare_actuals.py | 110 ++++++++++++++++++ 2 files changed, 157 insertions(+), 2 deletions(-) diff --git a/tools/trace_replay/run_android_retrace_local.py b/tools/trace_replay/run_android_retrace_local.py index 7d0b7635..307f72f3 100644 --- a/tools/trace_replay/run_android_retrace_local.py +++ b/tools/trace_replay/run_android_retrace_local.py @@ -4,6 +4,7 @@ import datetime import hashlib import json import math +import os import shutil import subprocess import sys @@ -95,8 +96,27 @@ def find_trace_apk(): return max(candidates, key=lambda path: path.stat().st_mtime) if candidates else None +# THE HOST SHELL, PER PLATFORM. This runner was written for Git Bash on Windows, where +# trace-replay-ci.sh has to be started through Git's bash.exe and every host path handed to it +# spelled /c/... . On Linux (WSL included - the device window's detachable `setsid nohup` runs +# live there) that bash.exe does not exist, so every case died with FileNotFoundError before the +# first adb call, and a /c/... spelling of a native path is not a path at all. trace-replay-ci.sh +# itself is POSIX shell (it converts with cygpath only when cygpath exists), so on a POSIX host +# the right answer is the system bash and the path unchanged. +def on_windows(): + return os.name == "nt" + + +def bash_executable(): + if on_windows(): + return "C:/Program Files/Git/bin/bash.exe" + return shutil.which("bash") or "/bin/bash" + + def bash_path(path): path = Path(path).resolve() + if not on_windows(): + return str(path) drive = path.drive.rstrip(":").lower() parts = path.parts[1:] return "/" + drive + "/" + "/".join(parts) @@ -187,7 +207,7 @@ def run_case(case, backend, extra_args=None, timeout_seconds=None, env_overrides alternate = None command = [ - "C:/Program Files/Git/bin/bash.exe", + bash_executable(), "android-plugin/trace-replay-ci.sh", "--apk-file", bash_path(apk), @@ -243,7 +263,9 @@ def run_case(case, backend, extra_args=None, timeout_seconds=None, env_overrides if env_overrides: command.extend(["--env", ";".join(env_overrides)]) env = dict(**__import__("os").environ) - env["PYTHON"] = "python" + # trace-replay-ci.sh reads its verdicts with "${PYTHON}". Git Bash on Windows has `python` + # on PATH; a POSIX host is only guaranteed the interpreter running this script. + env["PYTHON"] = "python" if on_windows() else sys.executable env["MSYS2_ARG_CONV_EXCL"] = "/data/*" if backend_info["use_angle"]: env["MOBILEGL_ESPRYT_USE_ANGLE"] = "1" @@ -309,6 +331,28 @@ def archive_repeat(archive_dir, case, backend, repeat_index): return destination +# WHAT ONE REPEAT LEAVES IN THE RESULT ROOT, and why it is cleared before the next one starts. +# trace-replay-ci.sh refreshes the logs on every run, but it writes result.json and the actual / +# diff PNGs only when the app produced them - a repeat whose app died before result.json (the +# bsl-esc-menu scudo abort), or whose adb dropped, leaves the PREVIOUS repeat's files in place. +# archive_repeat then copies them into this repeat's directory, and compare_actuals.py reports a +# repeat that rendered nothing as a passing, bit-identical one: exactly the false green gate 3's +# "three passes bit-identical" would read. A stale adb-disconnected.txt would likewise make the +# next repeat look like a disconnect. Goldens are inputs, not outputs, and are kept. +PER_REPEAT_OUTPUTS = ARCHIVED_PER_REPEAT + ("retrace.log", "adb-disconnected.txt") + + +def clear_repeat_outputs(case, backend): + result_dir = RESULT_ROOT / f"{safe_case(case['name'])}-{backend}" + if not result_dir.is_dir(): + return + for name in PER_REPEAT_OUTPUTS: + (result_dir / name).unlink(missing_ok=True) + for pattern in ("*-actual.png", "*-diff.png"): + for path in result_dir.glob(pattern): + path.unlink() + + def write_archive_manifest(archive_dir, args, backends, cases): """What this archive IS, recorded beside it rather than in the operator's shell history. @@ -694,6 +738,7 @@ def main(): # re-installing between them would put an installer inside the window and # would not change a byte of the input. extra.append("--reuse-fixture") + clear_repeat_outputs(resolved, backend) rc = run_case(resolved, backend, extra_args=extra, env_overrides=transport_env(args) + list(args.env), use_pbuffer=args.use_pbuffer) diff --git a/tools/trace_replay/test_compare_actuals.py b/tools/trace_replay/test_compare_actuals.py index 996538e9..2b399c3f 100644 --- a/tools/trace_replay/test_compare_actuals.py +++ b/tools/trace_replay/test_compare_actuals.py @@ -8,14 +8,18 @@ The summary half runs on a fabricated --archive-dir tree, because the failure it of the DIRECTORY, not of the images. """ import json +import os from pathlib import Path +import subprocess import sys import tempfile import unittest +from unittest import mock sys.path.insert(0, str(Path(__file__).resolve().parent)) import compare_actuals as ca import run_android_retrace_local as retrace +import trace_cases # The two closed forms the self-test derives. Written out here as literals rather than @@ -284,5 +288,111 @@ class Archive(unittest.TestCase): self.assertFalse(report["arms"][0]["all_bitwise_identical"]) +class RepeatIsolation(unittest.TestCase): + """The repeat loop itself: a repeat that rendered nothing must archive nothing. + + The Archive class above pins what archive_repeat copies. This pins what main() leaves in the + result root for it to copy: trace-replay-ci.sh writes result.json and the actual PNG only + when the app produced them, so without a clear between repeats the second repeat of a case + whose app died (bsl-esc-menu's scudo abort) archives the FIRST repeat's picture, and the + summary calls it a passing, bit-identical repeat - the false green gate 3 reads. + """ + + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="retrace-repeat-") + self.root = Path(self.temp.name) + apk = self.root / "pinned.apk" + apk.write_bytes(b"not really an apk") + self.patches = [ + mock.patch.object(retrace, "RESULT_ROOT", self.root / "result-root"), + mock.patch.object(retrace, "render_summary", lambda: None), + mock.patch.dict(os.environ, {"MOBILEGL_TRACE_APK": str(apk)}), + ] + for patch in self.patches: + patch.start() + + def tearDown(self): + for patch in reversed(self.patches): + patch.stop() + self.temp.cleanup() + + def test_a_repeat_whose_app_died_archives_no_stale_picture(self): + calls = [] + + def fake_run_case(case, backend, extra_args=None, timeout_seconds=None, + env_overrides=None, use_pbuffer=False): + calls.append(list(extra_args or [])) + arm = retrace.RESULT_ROOT / (retrace.safe_case(case["name"]) + "-" + backend) + arm.mkdir(parents=True, exist_ok=True) + # trace-replay-ci.sh refreshes the logs on every run, success or not ... + (arm / "logcat.txt").write_text("run %d\n" % len(calls), encoding="utf-8") + if len(calls) == 1: + # ... but only a run whose app lived writes result.json and the actual. + write_png(arm / (arm.name + "-actual.png"), 8, 4, solid(8, 4, 10)) + (arm / "result.json").write_text(json.dumps( + {"passed": True, "statusCode": 0, "backend": backend, "ssim": 1.0, + "ssimThreshold": 0.99, "matchedGoldenPath": "/x/g.png", + "mismatchPixels": 0, "cropX": 0, "cropY": 0, "cropWidth": 0, + "cropHeight": 0}), encoding="utf-8") + return 0 + return 1 + + archive = self.root / "archive" + argv = ["run_android_retrace_local.py", "--case", "OpenRA", "--backend", "DirectVulkan", + "--repeat", "2", "--archive-dir", str(archive)] + with mock.patch.object(retrace, "run_case", fake_run_case), \ + mock.patch.object(sys, "argv", argv): + self.assertEqual(retrace.main(), 1) + self.assertEqual(len(calls), 2) + arm = archive / "OpenRA-DirectVulkan" + self.assertTrue((arm / "repeat-01" / "result.json").is_file()) + self.assertTrue((arm / "repeat-01" / "OpenRA-DirectVulkan-actual.png").is_file()) + self.assertFalse((arm / "repeat-02" / "result.json").exists()) + self.assertFalse((arm / "repeat-02" / "OpenRA-DirectVulkan-actual.png").exists()) + report = ca.summarize(archive) + self.assertEqual(report["rows"][1]["error"], "no result.json") + self.assertFalse(report["arms"][0]["all_bitwise_identical"]) + self.assertFalse(report["arms"][0]["passed_all"]) + + +@unittest.skipIf(os.name == "nt", "the Git Bash spelling is the Windows branch of the same code") +class PosixHost(unittest.TestCase): + """The device window runs detached from WSL, so the runner has to start on a POSIX host. + + It used to hardcode Git Bash's `C:/Program Files/Git/bin/bash.exe` and spell every host path + /c/...: on Linux every case died with FileNotFoundError before its first adb call. + """ + + def test_run_case_starts_a_posix_bash_with_native_paths(self): + with tempfile.TemporaryDirectory(prefix="retrace-posix-") as directory: + root = Path(directory) + case = retrace.case_with_defaults(trace_cases.find_trace_case("OpenRA")) + fixtures = root / "fixtures" + fixtures.mkdir() + (fixtures / case["trace_archive"]).write_bytes(b"a real archive, not an LFS pointer") + write_png(fixtures / case["golden"], 8, 4, solid(8, 4, 0)) + apk = root / "pinned.apk" + apk.write_bytes(b"apk") + captured = {} + + def fake_run(command, cwd=None, env=None, **_): + captured.update(command=command, env=env) + return subprocess.CompletedProcess(command, 0) + + with mock.patch.object(retrace, "FIXTURES", fixtures), \ + mock.patch.object(retrace, "RESULT_ROOT", root / "result-root"), \ + mock.patch.object(retrace, "FIXTURE_ROOT", root / "fixture-root"), \ + mock.patch.dict(os.environ, {"MOBILEGL_TRACE_APK": str(apk)}), \ + mock.patch.object(retrace.subprocess, "run", fake_run): + self.assertEqual(retrace.run_case(case, "DirectVulkan"), 0) + command = captured["command"] + self.assertNotIn("Program Files", command[0]) + self.assertTrue(Path(command[0]).is_file(), command[0]) + self.assertEqual(command[command.index("--apk-file") + 1], str(apk.resolve())) + self.assertEqual(command[command.index("--golden") + 1], + str((fixtures / case["golden"]).resolve())) + self.assertEqual(captured["env"]["PYTHON"], sys.executable) + + if __name__ == "__main__": unittest.main(verbosity=2)