From 2d7c9b325a90ac8dbdf736018373486db8ea5e2b Mon Sep 17 00:00:00 2001 From: LuShadowX Date: Sat, 8 Aug 2026 09:10:20 +0530 Subject: [PATCH 1/3] Bring the README's platform, comparator and overhead claims up to date Windows is tested in CI as of #20, numpy arrays are compared as of #19, and recording has cost ~8x rather than 15-20x since the back-off work. --- README.md | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 90005bb..bb55c4d 100644 --- a/README.md +++ b/README.md @@ -40,11 +40,8 @@ here is the difference.** pip install nodrift ``` -Requires Python 3.9+. Unix only for now: the per-call timeout is portable as -of 0.1.2, but per-run temporary directories are still scrubbed with POSIX -paths, so on Windows identical code would compare unequal ([#17]). - -[#17]: https://github.com/LuShadowX/nodrift/issues/17 +Requires Python 3.9+. Tested in CI on Linux, macOS and Windows across Python +3.9, 3.11 and 3.13. ## Use @@ -142,10 +139,9 @@ most of it touches one file and needs one test. Start with the [good first issues][gfi]. The broad areas that need help: -- **Side-effect capture** — intercept `requests`, `sqlalchemy`, `open()` -- **Comparators** for types that need tolerance, e.g. numpy arrays -- **Performance** — recording currently costs 15-20x -- **Windows support** — replace the `SIGALRM` timeout +- **Side-effect capture** — intercept `requests`, `sqlalchemy` +- **Comparators** for types that need a policy, e.g. datetimes and UUIDs +- **Performance** — recording currently costs ~8x - **Framework adapters** beyond pytest The most valuable bug report of all is a **false positive**: if `nodrift check` From 048061565d5e63d01aa6243df3984b116c0bf845 Mon Sep 17 00:00:00 2001 From: LuShadowX Date: Sat, 8 Aug 2026 09:11:40 +0530 Subject: [PATCH 2/3] Let check compare two arbitrary refs check took one ref and always compared it against the working tree, so reviewing someone else's branch or auditing a release meant checking it out first. A second optional positional ref is now the candidate: nodrift check HEAD~1 HEAD With no second ref the working tree is still the candidate, which is the common case. Closes #11 --- src/nodrift/cli.py | 17 +++++++++++--- tests/test_end_to_end.py | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/nodrift/cli.py b/src/nodrift/cli.py index de2e476..fca6bdf 100644 --- a/src/nodrift/cli.py +++ b/src/nodrift/cli.py @@ -136,6 +136,8 @@ def cmd_check(args: argparse.Namespace) -> int: base_2 = os.path.join(results, "base2.json") head = os.path.join(results, "head.json") + candidate = getattr(args, "against", None) + print(f"nodrift: replaying {args.ref} ...", file=sys.stderr) _export(args.ref, stage) _replay(recording, stage, base_1, args.subdir) @@ -143,8 +145,14 @@ def cmd_check(args: argparse.Namespace) -> int: # nondeterministic and cannot support a claim either way. _replay(recording, stage, base_2, args.subdir) - print("nodrift: replaying working tree ...", file=sys.stderr) - _export_worktree(stage) + # With no second ref the candidate is the working tree, which is the + # common case: check what you are about to commit. + print(f"nodrift: replaying {candidate or 'working tree'} ...", + file=sys.stderr) + if candidate: + _export(candidate, stage) + else: + _export_worktree(stage) _replay(recording, stage, head, args.subdir) report = compare(base_1, head, base_2) @@ -233,7 +241,10 @@ def main(argv: list[str] | None = None) -> int: chk = sub.add_parser("check", help="compare a git ref against the working tree") chk.add_argument("ref", nargs="?", default="HEAD", - help="git ref to compare against (default HEAD)") + help="git ref to treat as the baseline (default HEAD)") + chk.add_argument("against", nargs="?", default=None, + help="second ref to compare with; defaults to the " + "working tree") chk.add_argument("--recording", "-r", default=None) chk.add_argument("--subdir", default="", help="path within the repo holding the package (e.g. src)") diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index d98ea3b..e19f060 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -473,3 +473,51 @@ def test_both_timeout_mechanisms_replay_a_recording_identically( stable = {key for key, value in first.items() if second.get(key) == value} assert stable, "replay produced nothing deterministic to compare" assert {k: with_watchdog[k] for k in stable} == {k: first[k] for k in stable} + + +def test_check_compares_two_arbitrary_refs(tmp_path): + """`nodrift check A B` must compare the two commits, not the worktree. + + Reviewing someone else's branch, or auditing a release, means comparing + two commits neither of which is checked out. + """ + repo = _git_repo(tmp_path) + _nodrift(repo, "record", "--package", "shapes") + + def commit(message): + subprocess.run(["git", "add", "-A"], cwd=str(repo), check=True, + capture_output=True) + subprocess.run( + ["git", "-c", "user.email=t@t", "-c", "user.name=t", + "commit", "-qm", message], + cwd=str(repo), check=True, capture_output=True, + ) + + source = (repo / "shapes.py").read_text() + (repo / "shapes.py").write_text( + source.replace('return "positive"', 'return "POSITIVE"')) + commit("change the label") + + # Put the working tree back to the original, so a comparison that used it + # instead of the second ref would wrongly report no change. + (repo / "shapes.py").write_text(source) + + changed = _nodrift(repo, "check", "HEAD~1", "HEAD") + assert changed.returncode == 1, changed.stdout + changed.stderr + assert "behave differently" in changed.stdout + + same = _nodrift(repo, "check", "HEAD~1", "HEAD~1") + assert same.returncode == 0, same.stdout + same.stderr + + +def test_check_still_defaults_to_the_working_tree(tmp_path): + repo = _git_repo(tmp_path) + _nodrift(repo, "record", "--package", "shapes") + + source = (repo / "shapes.py").read_text() + (repo / "shapes.py").write_text( + source.replace('return "positive"', 'return "POSITIVE"')) + + changed = _nodrift(repo, "check", "HEAD") + assert changed.returncode == 1, changed.stdout + changed.stderr + assert "working tree" in changed.stderr From 94c166ecdd8e6fba035291a04c8f0456c814f1e6 Mon Sep 17 00:00:00 2001 From: LuShadowX Date: Sat, 8 Aug 2026 09:20:10 +0530 Subject: [PATCH 3/3] Strip compiled bytecode from staged trees A repository with __pycache__ committed hands the replay bytecode compiled from other source. Python runs it in preference to the file git archive just materialised, so both versions behave identically and check reports no behaviour change while the code genuinely differs. Found while testing two-ref comparison: a tree staged from a commit containing POSITIVE replayed as positive, and check called it clean. The one-ref path had the same hole. Silent wrongness rather than an error, which is the worst failure this tool has. Compiled artifacts are now removed from every staged tree, exported or copied. The regression test commits stale bytecode deliberately and fails without the fix. --- src/nodrift/cli.py | 24 ++++++++++++++++++++++++ tests/test_end_to_end.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/nodrift/cli.py b/src/nodrift/cli.py index fca6bdf..168e510 100644 --- a/src/nodrift/cli.py +++ b/src/nodrift/cli.py @@ -56,6 +56,28 @@ def _git(*argv: str, cwd: str | None = None) -> str: ).stdout.strip() +def _strip_bytecode(root: str) -> None: + """Delete compiled bytecode from a staged tree. + + A repository with `__pycache__` committed — or a stray `.pyc` — hands the + replay bytecode compiled from *other* source. Python will run it in + preference to the file we just materialised, so `check` reports no + behaviour change while the two versions genuinely differ. Silent, and + worse than an error. + """ + for dirpath, dirnames, filenames in os.walk(root): + if os.path.basename(dirpath) == "__pycache__": + shutil.rmtree(dirpath, ignore_errors=True) + dirnames[:] = [] + continue + for name in filenames: + if name.endswith((".pyc", ".pyo")): + try: + os.remove(os.path.join(dirpath, name)) + except OSError: + pass + + def _export(ref: str, dest: str) -> None: """Materialise `ref` into `dest` (which is emptied first).""" if os.path.exists(dest): @@ -66,6 +88,7 @@ def _export(ref: str, dest: str) -> None: capture_output=True, check=True, ).stdout subprocess.run(["tar", "-x", "-C", dest], input=archive, check=True) + _strip_bytecode(dest) def _export_worktree(dest: str) -> None: @@ -80,6 +103,7 @@ def _export_worktree(dest: str) -> None: target = os.path.join(dest, rel) os.makedirs(os.path.dirname(target), exist_ok=True) shutil.copy2(src, target) + _strip_bytecode(dest) def _replay(recording: str, source_root: str, out: str, sub: str) -> None: diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index e19f060..a7f3978 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -521,3 +521,38 @@ def test_check_still_defaults_to_the_working_tree(tmp_path): changed = _nodrift(repo, "check", "HEAD") assert changed.returncode == 1, changed.stdout + changed.stderr assert "working tree" in changed.stderr + + +def test_committed_bytecode_cannot_shadow_the_staged_source(tmp_path): + """Stale `.pyc` in a ref must not be executed in place of its source. + + A repository with `__pycache__` committed hands the replay bytecode + compiled from other source. Python prefers it to the file just + materialised, so both versions behave identically and `check` reports no + change while the code genuinely differs — a confident wrong answer. + """ + repo = _git_repo(tmp_path) + _nodrift(repo, "record", "--package", "shapes") + + source = (repo / "shapes.py").read_text() + (repo / "shapes.py").write_text( + source.replace('return "positive"', 'return "POSITIVE"')) + + # Recording imported the original module, so __pycache__ now holds + # bytecode for the *old* source. Committing it is what sets the trap. + cache = repo / "__pycache__" + assert cache.exists(), "expected recording to have left bytecode behind" + subprocess.run(["git", "add", "-A"], cwd=str(repo), check=True, + capture_output=True) + subprocess.run( + ["git", "-c", "user.email=t@t", "-c", "user.name=t", + "commit", "-qm", "commit the bytecode too"], + cwd=str(repo), check=True, capture_output=True, + ) + + result = _nodrift(repo, "check", "HEAD~1", "HEAD") + assert result.returncode == 1, ( + "stale bytecode hid a real behaviour change\n" + + result.stdout + result.stderr + ) + assert "classify" in result.stdout