Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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`
Expand Down
41 changes: 38 additions & 3 deletions src/nodrift/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -136,15 +160,23 @@ 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)
# Second identical pass: anything that disagrees with itself here is
# 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)
Expand Down Expand Up @@ -233,7 +265,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)")
Expand Down
83 changes: 83 additions & 0 deletions tests/test_end_to_end.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,3 +473,86 @@ 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


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
Loading