From 07a73abf918711bb776eec9b524bae58dafbd826 Mon Sep 17 00:00:00 2001 From: blasty Date: Sun, 26 Jul 2026 21:02:47 +0200 Subject: tests: run the scenario suite on a scratch copy, not the tracked target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite edits the database — defines code, undefines items, renames, comments — and IDA saves all of it. Running that against targets/echo.i64 meant every run inherited the last one's damage. That cost real time twice. decomp_follow_self "started failing" with no code change, and stayed failing until the .i64 was deleted; an edit-position check looked flaky about one run in three and I nearly reported it as an async race. Both were the database drifting. A suite whose result depends on its own history cannot be trusted to accuse the code — and it had been quietly laundering bad conclusions for however long. Now the suite copies the binary into a temp dir and seeds it from a golden database (.pristine.i64) that nothing ever writes back to. Every run starts from identical bytes; the tracked target is never opened. The golden copy is built once, on first run, by analysing and saving before any scenario runs — so it costs one analysis rather than one per run. Rebuilt automatically if the binary is newer. Verified: two consecutive full runs both 209/0; targets/echo.i64 no longer exists after a run; a deliberately corrupted targets/echo.i64 is ignored completely (11/0 with junk in place, and the junk untouched afterwards); no temp directories leak. The other suites were already clean for the same reason, by different means: test_blob_ui builds a throwaway binary, test_project_ui stages copies, and test_thumb_ui deletes the .i64 before each phase because the T flag and the segment's bitness are saved in it. --- .gitignore | 1 + TODO | 28 +++++++++++++--------------- tests/test_scenarios.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index b1d85aa..6ff0889 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ build/ # large analysis targets (copied in, not source) targets/ bin/ +*.pristine.i64 diff --git a/TODO b/TODO index 64d2b26..90da191 100644 --- a/TODO +++ b/TODO @@ -72,21 +72,19 @@ done-ish: -## The scenario suite mutates a PERSISTENT database +## Test hygiene (fixed, worth remembering) -tests/test_scenarios.py runs against targets/echo.i64 and every edit it makes is -saved there. A scenario that undefines an instruction leaves that instruction -undefined for every later run — decomp_follow_self started failing "for no -reason" and stayed failing until the .i64 was deleted and re-analysed. +tests/test_scenarios.py used to run against targets/echo.i64 and IDA saved every +edit it made, so each run inherited the previous run's damage. It cost real time +twice: decomp_follow_self "started failing" with no code change (an earlier +scenario had undefined an instruction), and an edit-position check looked flaky +one run in three, which nearly got written up as an async race. -That also poisoned an investigation: an `edit_keeps_view` check looked flaky -(cursor jumping 0x20c6 -> 0x2094 about one run in three) and was almost -certainly the database drifting between runs, not a race. Any conclusion drawn -from repeated runs of a mutating scenario is suspect. +Now: the suite copies the binary into a temp dir and seeds it from a golden +database (.pristine.i64, built once, never written back). Every run +starts from identical bytes and the tracked target is never touched. -Worth fixing properly: either give the suite a scratch copy of the binary per -run, or have mutating scenarios undo themselves. Until then, `rm targets/*.i64` -before trusting a failure that appeared without a code change. - -Coverage for "an edit must not move the view" lives in tests/test_blob_ui.py, -which builds its own throwaway binary and can mutate freely. +The general rule this came from: a suite whose result depends on its own history +can't be trusted to accuse the code. tests/test_blob_ui.py builds a throwaway +binary; test_project_ui.py stages copies; test_thumb_ui.py deletes the .i64 +before each phase because the T flag and segment bitness are SAVED in it. diff --git a/tests/test_scenarios.py b/tests/test_scenarios.py index 53dad91..09e1ac5 100644 --- a/tests/test_scenarios.py +++ b/tests/test_scenarios.py @@ -20,7 +20,9 @@ import asyncio import fnmatch import os import re +import shutil import sys +import tempfile import traceback sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -2270,7 +2272,49 @@ async def s_func_banners(c: Ctx): # --------------------------------------------------------------------------- # # Runner # --------------------------------------------------------------------------- # +async def _build_pristine(binary, cache): + """Analyse ``binary`` once and keep the resulting database as a golden copy. + + Costs one full analysis, then every later run starts from it instead of + re-analysing. + """ + print(f" (building pristine database for {os.path.basename(binary)}\u2026)") + app = IdaTui(open_path=binary, keepalive=False) + async with app.run_test(size=(140, 44)) as pilot: + for _ in range(6000): + await pilot.pause(0.05) + if app._func_index is not None and app._func_index.complete: + break + app.program.client.call("idb_save", timeout=600.0) + db = binary + ".i64" + if os.path.exists(db): + shutil.copy2(db, cache) + + async def run(binary, only=None): + # The suite EDITS the database — it defines code, undefines items, renames + # and comments — and IDA saves those edits. Run that against the tracked + # target and every run inherits the last one's damage: decomp_follow_self + # started failing with no code change because an earlier scenario had + # undefined an instruction, and a position check looked flaky one run in + # three for the same reason. A suite whose result depends on its own history + # can't be trusted to accuse the code. + # + # So: work on a scratch copy, seeded from a golden database that nothing + # ever writes back to. + with tempfile.TemporaryDirectory(prefix="idatui-pilot-") as scratch: + target = os.path.join(scratch, os.path.basename(binary)) + shutil.copy2(binary, target) + cache = binary + ".pristine.i64" + if not (os.path.exists(cache) + and os.path.getmtime(cache) >= os.path.getmtime(binary)): + await _build_pristine(target, cache) + if os.path.exists(cache): + shutil.copy2(cache, target + ".i64") + await _run_on(target, only) + + +async def _run_on(binary, only=None): # Own idalib worker: opens the binary in-process over a unix socket. app = IdaTui(open_path=binary, keepalive=False) async with app.run_test(size=(140, 44)) as pilot: -- cgit v1.3.1-sl0p