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
8 changes: 8 additions & 0 deletions src/nodrift/compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ def compare(path_a: str, path_b: str, path_a2: str | None = None) -> dict:
Any record that disagrees with itself across those two runs is inherently
nondeterministic (embedded ids, clocks, randomness) and is quarantined
rather than reported, since no claim about it can be made either way.

Quarantining is the whole answer here, not a placeholder for one. Making
clock- and uuid-reading functions comparable by normalising them was
considered and rejected in issue #12; the reasoning, and the specific
genuine bugs each flavour of normalisation would hide, are recorded on
`replay._install_determinism_controls`. What a quarantined record costs
the user is visibility, so the report carries `quarantined_targets` and
`check` prints the names.
"""
with open(path_a) as fh:
a = json.load(fh)
Expand Down
36 changes: 35 additions & 1 deletion src/nodrift/replay.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,41 @@ def run(recording_path: str, deterministic: bool) -> dict:


def _install_determinism_controls() -> None:
"""Neutralise the common sources of run-to-run variation."""
"""Neutralise the common sources of run-to-run variation.

Deliberately absent: freezing the clock and seeding `uuid4`. Functions
that read `datetime.now()`, `time.time()` or `uuid4()` disagree with
themselves across the two baseline passes, so `compare` quarantines them
and `check` names them. That is a coverage hole, and a large one — on a
module where every function touches a clock or an id, 8 of 9 functions go
unchecked. Closing it by normalising here was considered and rejected
(issue #12). The measurements behind that:

* Freezing every clock reader to one constant hides a `now()` -> `utcnow()`
swap, hides elapsed-time logic being deleted (every duration becomes
zero), and — worst — turns TTL and cache-expiry branches into dead code,
so a version that removed the refresh path entirely compares equal to one
that kept it. Normalisation is not output scrubbing; it is an
intervention in the program under test, and it can silence exactly the
branch the user wanted compared.
* Seeding `uuid4` has no safe granularity. Seed once per process and the
records share one stream, so a candidate that changes how many ids one
function mints shifts every later record and reports byte-identical
functions as changed — a false positive, which this tool weighs as the
worse error. Reseed per record and that goes away, but a fresh `uuid4()`
regressing into a module-level constant then compares equal.
* Scrubbing datetimes/UUIDs out of the fingerprint instead is worse still:
it hides any change *computed from* a clock, e.g. `timedelta(seconds=n)`
becoming `timedelta(minutes=n)`.

Quarantine is incomplete but never wrong: it makes no claim, so it cannot
make a false one, and since #12 part 1 it says out loud which functions it
dropped. Every normalisation above trades that for a confident answer that
is sometimes wrong. The honest fix for an unchecked clock-reader is to
inject the clock, which is the user's call and not something to fake on
their behalf. If this is revisited, the bar is: name the genuine change
each new control would hide, and show a test that still catches it.
"""
import random
import os

Expand Down
Loading