From 63ba3fc22b6b0fcd492836088d922ef8ef91e24f Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Mon, 31 Aug 2026 16:06:06 -0700 Subject: [PATCH] test: gate the cache-name conflict, and say so where it happens (#706) The normaliser reports a `[conflict]` when both casings of a cache name exist as distinct files and refuses to choose -- picking between two fetches of the same reference is not a script's call. But the recipes discard its exit code, on purpose: normalising must not turn a failing validation green (#697). So the condition was one line in the middle of a long validation log and nothing else. Two changes, and the first matters more than the issue suggested. **The committed tree is now gated on every machine.** `test_reference_cache_names_are_case_exact.py` reads `os.listdir(REFERENCES_CACHE)` -- the filesystem. macOS cannot hold `DOI_x.md` and `doi_x.md` at once, so on a developer machine the pair is invisible and the divergence appears only on Linux. That is #690's lesson stated as a test: a green local run is not a green CI run. The new check reads GIT'S INDEX instead, which records the name it was told regardless of what the filesystem will store, so the clash is caught everywhere. Mutation-checked by staging a clashing name directly into the index with `git update-index --cacheinfo` -- a state macOS cannot represent on disk, which is exactly why the index is the right thing to read. **A conflict is announced rather than buried.** The script prints a trailing banner naming the count and what a human has to decide. The exit code is deliberately left as it was: the recipes' `exit $code` is the validator's, and that is correct. The banner is tested by stubbing `rename`, because the real conflict needs two files whose names differ only by case -- the filesystem, not the code, is what makes it unreachable here. Both directions: a conflict prints it, no conflict does not, and removing the banner reds the first. Co-Authored-By: Claude Opus 5 --- scripts/normalize_cache_names.py | 15 ++++ ...che_names_are_normalised_after_fetching.py | 39 +++++++++++ tests/test_no_case_conflicting_cache_pairs.py | 69 +++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 tests/test_no_case_conflicting_cache_pairs.py diff --git a/scripts/normalize_cache_names.py b/scripts/normalize_cache_names.py index 535310e7..1b713c19 100644 --- a/scripts/normalize_cache_names.py +++ b/scripts/normalize_cache_names.py @@ -138,6 +138,21 @@ def main(argv: list[str] | None = None) -> int: # A conflict is left for a human: both casings exist as distinct files, and # choosing between two fetches of the same reference is not this script's # call. + # + # Said loudly, because the recipes that run this DISCARD its exit code on + # purpose -- normalising must not turn a failing validation green (#697) -- + # so a conflict would otherwise be one line in the middle of a long + # validation log (#706). The committed tree is separately gated by + # tests/test_no_case_conflicting_cache_pairs.py, which reads git's index + # rather than the filesystem so the clash is visible on macOS too. + if conflicts: + print( + f"\n⚠️ {conflicts} cache name conflict(s): both casings exist as " + f"distinct files, so on a case-sensitive filesystem at most one is " + f"reachable. Nothing was renamed for those; a human has to choose " + f"which fetch to keep (#706).", + file=sys.stderr, + ) return 1 if conflicts else 0 diff --git a/tests/test_cache_names_are_normalised_after_fetching.py b/tests/test_cache_names_are_normalised_after_fetching.py index 3ad59813..2985f6ac 100644 --- a/tests/test_cache_names_are_normalised_after_fetching.py +++ b/tests/test_cache_names_are_normalised_after_fetching.py @@ -224,3 +224,42 @@ def test_the_check_can_actually_fail(tmp_path): "survived, so `test_a_leftover_temporary_is_never_clobbered` would pass " "with or without the guard it is meant to defend" ) + + +def test_a_conflict_is_announced_where_a_reader_will_see_it(tmp_path, capsys, monkeypatch): + """A `[conflict]` must not be one line in the middle of a validation log. + + The recipes discard this script's exit code on purpose -- normalising must + not turn a failing validation green (#697) -- so the exit code cannot be the + signal. The banner is (#706). + + Driven by stubbing `rename`, because the real conflict needs two files whose + names differ only by case and macOS cannot hold both; the filesystem, not + the code, is what makes it unreachable here. + """ + module = _script() + (tmp_path / "DOI_10.9999_clash.md").write_text("from the fetcher", encoding="utf-8") + + def _always_conflicts(current, wanted): + return f"[conflict] {current.name}: {wanted.name} already exists and differs" + + monkeypatch.setitem(module, "rename", _always_conflicts) + + code = module["main"](["--cache-dir", str(tmp_path)]) + + captured = capsys.readouterr() + assert code == 1, "a conflict must still be a non-zero exit from the script itself" + assert "cache name conflict" in captured.err, captured.err + assert "#706" in captured.err, captured.err + + +def test_no_conflict_means_no_banner(tmp_path, capsys): + """The other direction, so the test above cannot pass vacuously.""" + module = _script() + (tmp_path / "DOI_10.9999_fine.md").write_text("cached", encoding="utf-8") + + code = module["main"](["--cache-dir", str(tmp_path)]) + + captured = capsys.readouterr() + assert code == 0, captured.err + assert "cache name conflict" not in captured.err, captured.err diff --git a/tests/test_no_case_conflicting_cache_pairs.py b/tests/test_no_case_conflicting_cache_pairs.py new file mode 100644 index 00000000..70cc4074 --- /dev/null +++ b/tests/test_no_case_conflicting_cache_pairs.py @@ -0,0 +1,69 @@ +"""Two cache files differing only by prefix case cannot both be reachable (#706). + +`scripts/normalize_cache_names.py` renames `DOI_x.md` to `doi_x.md`, and when +BOTH already exist as distinct tracked files it reports a `[conflict]` and +refuses — choosing between two fetches of the same reference is not a script's +call. But the justfile recipes discard the normaliser's exit code (deliberately: +normalising must not turn a failing validation green), so that conflict is one +printed line and nothing else. + +**Why the existing case-exactness test does not close this.** +`test_reference_cache_names_are_case_exact.py` reads +`os.listdir(REFERENCES_CACHE)` — the filesystem. macOS cannot hold `DOI_x.md` +and `doi_x.md` at once, so on a developer machine the pair is invisible and the +divergence only appears on Linux. That is #690's lesson stated as a test: a +green local run is not a green CI run. + +Reading git's index instead makes the check case-exact on every machine, because +git records the name it was told regardless of what the filesystem will store. +""" + +from __future__ import annotations + +import collections +import pathlib +import subprocess + +REPO = pathlib.Path(__file__).parent.parent + + +def _tracked_cache_names() -> list[str]: + """Cache filenames as GIT records them, not as the filesystem reports them.""" + result = subprocess.run( + ["git", "ls-files", "-z", "--", "references_cache/"], + cwd=REPO, + capture_output=True, + text=True, + timeout=120, + check=True, + ) + # -z, because a path containing a space or a newline would otherwise split + # into several bogus entries -- the whitespace-splitting trap. + return [entry.rsplit("/", 1)[-1] for entry in result.stdout.split("\0") if entry] + + +def test_there_are_tracked_cache_files_to_check(): + """A finder that found nothing would make the check below vacuous.""" + names = _tracked_cache_names() + assert len(names) >= 400, f"only {len(names)} tracked cache files; the listing broke" + + +def test_no_two_tracked_caches_differ_only_by_case(): + """One reference must not have two files whose names differ only in case. + + On a case-sensitive filesystem both exist and only one is reachable, so the + other is a fetch nothing can read -- and which one wins depends on which + casing the citation happens to use. + """ + by_folded = collections.defaultdict(list) + for name in _tracked_cache_names(): + by_folded[name.casefold()].append(name) + + clashes = {folded: sorted(names) for folded, names in by_folded.items() if len(set(names)) > 1} + assert clashes == {}, ( + "these cache files differ only by case, so on a case-sensitive " + "filesystem both exist and at most one is reachable (#706). The " + "normaliser reports this as a [conflict] and refuses to choose; a human " + "has to decide which fetch to keep:\n " + + "\n ".join(f"{folded}: {names}" for folded, names in sorted(clashes.items())) + )