From d10d51e8dd833c6115999b7fd74cc5d290ed344f Mon Sep 17 00:00:00 2001 From: LuShadowX Date: Sat, 8 Aug 2026 08:53:51 +0530 Subject: [PATCH 1/3] Scrub temp paths on Windows too, and put Windows in CI The per-run temp directory patterns were POSIX-only: built from /-separated strings and ending in /[^/]+. A Windows temp path is C:\Users\\AppData\Local\Temp\, which none of them could match, so the varying segment survived into recorded write paths and two runs of identical code compared unequal. That is false-positive cause 4 from 0.1.0, returning on a new platform. Separators are normalised before matching and on the way out, so a path is recorded the same way whichever OS wrote it. AppData and Windows\Temp are matched explicitly and case-insensitively, so a recording made on another machine scrubs too. windows-latest joins the test matrix, since the claim is not worth making without it. Closes #17 --- .github/workflows/ci.yml | 2 +- src/nodrift/sideeffects.py | 19 ++++++++++++--- tests/test_sideeffects.py | 49 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4ea1f7..4733141 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] python: ["3.9", "3.11", "3.13"] steps: diff --git a/src/nodrift/sideeffects.py b/src/nodrift/sideeffects.py index ab5884c..aaac58d 100644 --- a/src/nodrift/sideeffects.py +++ b/src/nodrift/sideeffects.py @@ -22,13 +22,23 @@ import re import tempfile +def _slash(text: str) -> str: + """Use one separator everywhere, so one set of patterns fits both OSes.""" + return text.replace("\\", "/") + + # A per-run temporary directory, plus the one random segment inside it. # macOS puts temp files under /var/folders/<2 chars>//T/, so matching a # fixed prefix is not enough — the varying part is deeper than it looks. +# Windows hides it under the user's AppData, which `gettempdir()` covers on +# the running machine; the explicit pattern also catches a recording made +# elsewhere, and is case-insensitive because Windows paths are. _TEMP_ROOTS = [ r"/private/var/folders(?:/[^/]+){1,2}/T", r"/var/folders(?:/[^/]+){1,2}/T", - re.escape(tempfile.gettempdir()), + re.escape(_slash(tempfile.gettempdir())), + r"(?i:[A-Z]:/Users/[^/]+/AppData/Local/Temp)", + r"(?i:[A-Z]:/Windows/Temp)", r"/private/tmp", r"/tmp", ] @@ -90,10 +100,13 @@ def normalise(path: str, root: str | None = None) -> str: root = root or os.getcwd() try: if os.path.commonpath([os.path.abspath(text), root]) == root: - return os.path.relpath(os.path.abspath(text), root) + # Forward slashes on the way out too: a path recorded on one OS + # must not differ from the same path recorded on another purely + # by separator. + return _slash(os.path.relpath(os.path.abspath(text), root)) except (ValueError, OSError): pass - return _TEMPISH.sub(r"\1/", text) + return _TEMPISH.sub(r"\1/", _slash(text)) class WriteWatcher: diff --git a/tests/test_sideeffects.py b/tests/test_sideeffects.py index b9d9a37..96131a7 100644 --- a/tests/test_sideeffects.py +++ b/tests/test_sideeffects.py @@ -158,3 +158,52 @@ def test_write_change_is_detected_though_return_value_is_identical(tmp_path): "a changed file write went unnoticed; the return value is 'ok' either way" ) assert any("save" in target for target in report["changed"]) + + +# -------------------------------------------------------------------------- +# path scrubbing across operating systems +# -------------------------------------------------------------------------- + +def test_windows_temp_directories_are_scrubbed(): + """The per-run segment has to go, whichever OS wrote the path. + + Cause 4 of the false positives fixed in 0.1.0 was a per-run temp directory + surviving into a recorded write path. The patterns that fixed it were + POSIX-only, so on Windows two runs of identical code compared unequal. + Asserted against literal Windows paths so it holds on any host. + """ + from nodrift.sideeffects import _TEMPISH, _slash + + def scrub(path): + return _TEMPISH.sub(r"\1/", _slash(path)) + + first = scrub(r"C:\Users\lu\AppData\Local\Temp\nodrift-a1b2\out.txt") + second = scrub(r"C:\Users\lu\AppData\Local\Temp\nodrift-z9y8\out.txt") + assert first == second == "C:/Users/lu/AppData/Local/Temp//out.txt" + + # Drive letter and casing vary on Windows; the pattern must not care. + assert scrub(r"d:\users\lu\appdata\local\temp\xyz\out.txt").endswith( + "//out.txt" + ) + assert scrub(r"C:\Windows\Temp\abc\log.txt") == "C:/Windows/Temp//log.txt" + + +def test_posix_temp_directories_are_still_scrubbed(): + """The Windows patterns must not have displaced the existing ones.""" + from nodrift.sideeffects import _TEMPISH, _slash + + def scrub(path): + return _TEMPISH.sub(r"\1/", _slash(path)) + + assert scrub("/tmp/nodrift-a1b2/out.txt") == "/tmp//out.txt" + assert ( + scrub("/private/var/folders/xy/hash1234/T/nodrift-a1/out.txt") + == "/private/var/folders/xy/hash1234/T//out.txt" + ) + + +def test_paths_inside_the_root_come_back_with_forward_slashes(tmp_path): + """Recordings must not differ by separator alone.""" + root = str(tmp_path) + nested = os.path.join(root, "pkg", "data", "out.txt") + assert normalise(nested, root=root) == "pkg/data/out.txt" From e2ecf5bafd721252bb219431c6615fc49cae11e6 Mon Sep 17 00:00:00 2001 From: LuShadowX Date: Sat, 8 Aug 2026 08:59:21 +0530 Subject: [PATCH 2/3] Run the fixture's pytest from inside the project directory Given an absolute path, pytest walks up looking for a rootdir. On Windows that reaches C:\ and dies on the permission-denied "Documents and Settings" junction, so every test that spawns a pytest subprocess failed on windows-latest with Python 3.9. --- tests/test_end_to_end.py | 6 +++++- tests/test_xdist.py | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index 32b271c..d98ea3b 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -89,8 +89,12 @@ def recorded(tmp_path): recording = str(tmp_path / "recording.pkl") proc = subprocess.run( - [sys.executable, "-m", "pytest", str(project), "-q", "-p", "no:cacheprovider", + [sys.executable, "-m", "pytest", ".", "-q", "-p", "no:cacheprovider", "--nodrift", "toy", "--nodrift-out", recording], + # Run from inside the project. Given an absolute path, pytest walks up + # looking for a rootdir, and on Windows that reaches C:\ and dies on + # the permission-denied "Documents and Settings" junction. + cwd=str(project), capture_output=True, text=True, env=dict(os.environ, PYTHONPATH=str(project)), ) diff --git a/tests/test_xdist.py b/tests/test_xdist.py index def7a7f..2f75cf2 100644 --- a/tests/test_xdist.py +++ b/tests/test_xdist.py @@ -85,8 +85,11 @@ def test_parallel_recording_is_not_empty(tmp_path): out = str(tmp_path / "rec.pkl") proc = subprocess.run( - [sys.executable, "-m", "pytest", str(project), "-q", "-p", "no:cacheprovider", + [sys.executable, "-m", "pytest", ".", "-q", "-p", "no:cacheprovider", "-n", "2", "--nodrift", "calc", "--nodrift-out", out], + # See the note in test_end_to_end.py: an absolute path sends pytest's + # rootdir search up to C:\ on Windows. + cwd=str(project), capture_output=True, text=True, env=dict(os.environ, PYTHONPATH=str(project)), ) From 2f7349c88a82869d2d7479d3e28fbcd1e731a56a Mon Sep 17 00:00:00 2001 From: LuShadowX Date: Sat, 8 Aug 2026 09:04:51 +0530 Subject: [PATCH 3/3] Run the side-effect fixture's pytest from inside the project too Same Windows rootdir problem as the other two fixtures. --- tests/test_sideeffects.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_sideeffects.py b/tests/test_sideeffects.py index 96131a7..9184478 100644 --- a/tests/test_sideeffects.py +++ b/tests/test_sideeffects.py @@ -133,8 +133,11 @@ def test_write_change_is_detected_though_return_value_is_identical(tmp_path): recording = str(tmp_path / "rec.pkl") proc = subprocess.run( - [sys.executable, "-m", "pytest", str(project), "-q", "-p", "no:cacheprovider", + [sys.executable, "-m", "pytest", ".", "-q", "-p", "no:cacheprovider", "--nodrift", "app", "--nodrift-out", recording], + # See the note in test_end_to_end.py: an absolute path sends pytest's + # rootdir search up to C:\ on Windows. + cwd=str(project), capture_output=True, text=True, env=dict(os.environ, PYTHONPATH=str(project)), )