From c8bab3c3565c58ef484e234bde73dee6e1cc9dbc Mon Sep 17 00:00:00 2001 From: Ismar <1242091+ichoosetoaccept@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:53:02 +0200 Subject: [PATCH 1/4] test: tolerate copier's temp-clone cleanup race on a dirty tree `poe test` failed whenever the working tree was dirty and passed when it was clean, with the failure landing on whichever test happened to render first. One unrelated newline reproduced it. CI never saw it, because CI always checks out clean -- so the suite was green exactly when nobody needed it and red exactly when somebody was mid-change, with no signal separating it from a real failure. On a dirty tree copier takes its dirty-file overlay path, which does extra git work inside the temp clone it makes of this repo. Something still holds a handle under that clone's `.git` when `_cleanup` calls `rmtree(ignore_errors=False)`, so copier exits non-zero after every file has already been written. The render succeeds; only the cleanup fails. `generate_project` now tolerates that one exit, narrowly in three ways: stderr must name a path copier itself calls `copier._vcs.clone.*`, it must carry an rmtree/Errno 66 signature, and the destination must actually contain a rendered `pyproject.toml`. A render that genuinely failed writes no pyproject.toml, and every other non-zero exit still fails the test with copier's own stderr. A broader "ignore non-zero exits" would be a liability in a suite whose whole job is asserting on rendered output. Fast suite on a dirty tree: 202 passed. The same tree previously gave 2 failed, 13 passed in TestTemplateUpdateCheck alone. Closes DOT-606 --- tests/conftest.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0b7fbbb..1f30e32 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -70,12 +70,37 @@ def generate_project( cmd.extend(["-d", f"{key}={value}"]) result = subprocess.run(cmd, capture_output=True, text=True, check=False) - if result.returncode != 0: + if result.returncode != 0 and not _is_temp_clone_cleanup_failure(result.stderr, tmp_path): pytest.fail(f"Copier failed: {result.stderr}") return tmp_path +def _is_temp_clone_cleanup_failure(stderr: str, dst: Path) -> bool: + """True if copier rendered successfully but crashed clearing its own temp clone (DOT-606). + + On a dirty working tree copier takes its dirty-file overlay path, which does extra git + work inside the temp clone it made of this repo. Something still holds a handle under + that clone's `.git` when `_cleanup` calls `rmtree(..., ignore_errors=False)`, so copier + exits non-zero *after* every file has been written. Observed on macOS with copier 9.17.x. + + The effect is a test suite that is green in CI (which always checks out clean) and red + for anyone mid-change — with the failure landing on whichever test happens to render + first, so it reads as flakiness rather than as one deterministic bug. Re-running the + "failed" test alone usually passes, which sends triage down the wrong path entirely. + + Deliberately narrow. This tolerates only an `rmtree` of a path copier itself names + `copier._vcs.clone.*`, and only when the destination actually received a rendered + project. A render that genuinely failed leaves no `pyproject.toml`, and any other + non-zero exit still fails the test with copier's own stderr. + """ + if "copier._vcs.clone" not in stderr: + return False + if not any(marker in stderr for marker in ("Directory not empty", "rmtree", "Errno 66")): + return False + return (dst / "pyproject.toml").is_file() + + def _cache_key(answers: dict, project_type: str) -> str: """Create a stable cache key from answers dict and project type.""" merged = {**answers, "project_type": project_type} From cc625d78efdc70856897e944dbfb8607dd393cf4 Mon Sep 17 00:00:00 2001 From: Ismar <1242091+ichoosetoaccept@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:14:48 +0200 Subject: [PATCH 2/4] test: pin the boundary of the temp-clone cleanup tolerance `_is_temp_clone_cleanup_failure` is the only place in the suite that suppresses a non-zero copier exit, and it shipped with nothing testing where it stops. Both directions of drift are silent: widen it and a genuine render failure passes as a green test; narrow it and the dirty-tree failures it exists to absorb come back. The integration tests cannot catch either, because they only see whichever exit copier produces on the machine running them -- on a clean tree that is exit 0 and the helper is never consulted. Covers each accepted marker in isolation (`Directory not empty`, `rmtree`, `Errno 66`) so no case leans on another's signature, plus all three rejection gates: an OSError outside copier's own temp clone, a real failure inside the clone with no cleanup signature, and cleanup noise over a destination that never received a render. Confirmed to discriminate: replacing the rendered-`pyproject.toml` gate with `return True` fails `test_rejects_when_nothing_was_rendered`. Reported by Greptile on #348. --- tests/test_template.py | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/test_template.py b/tests/test_template.py index d4e808a..15e0b4f 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -13,7 +13,7 @@ import pytest import yaml -from conftest import REPO_ROOT, generate_project +from conftest import REPO_ROOT, _is_temp_clone_cleanup_failure, generate_project if TYPE_CHECKING: from pathlib import Path @@ -2652,3 +2652,43 @@ def test_semantic_release_writes_gitlab_urls_for_selfhosted(self, tmp_path: Path f"Underscored repo slug `die_zeit` leaked into CHANGELOG.md — semantic-release " f"appears to be using python_package_import_name instead of the repo slug:\n{changelog}" ) + + +class TestTempCloneCleanupTolerance: + """Pin the boundary of `conftest._is_temp_clone_cleanup_failure` (DOT-606). + + That helper is the one place in the suite where a non-zero copier exit is suppressed, + and both directions of drift are silent. Widen it and a genuine render failure passes + as a green test; narrow it and the dirty-tree failures it exists to absorb come back. + Neither shows up in the integration tests, which only ever see whichever exit copier + happens to produce on the machine running them — on a clean tree, that is exit 0 and + this helper is never consulted at all. + """ + + CLONE_CLEANUP_STDERR = "OSError: [Errno 66] Directory not empty: '/tmp/copier._vcs.clone.ab12cd34/.git'" + + @staticmethod + def _with_render(tmp_path: Path) -> Path: + """A destination that received a project: the render-succeeded half of the gate.""" + (tmp_path / "pyproject.toml").write_text('[project]\nname = "rendered"\n') + return tmp_path + + @pytest.mark.parametrize("marker", ["Directory not empty", "rmtree", "Errno 66"]) + def test_accepts_each_cleanup_marker(self, tmp_path: Path, marker: str) -> None: + """Each accepted signature is tested alone — no case leans on another's marker.""" + stderr = f"copier._vcs.clone.ab12cd34 cleanup failed: {marker}" + assert _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) + + def test_rejects_failure_outside_copiers_temp_clone(self, tmp_path: Path) -> None: + """Same OSError, a path copier did not create — could be the template's own tree.""" + stderr = "OSError: [Errno 66] Directory not empty: '/some/other/path/.git'" + assert not _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) + + def test_rejects_clone_path_without_a_cleanup_marker(self, tmp_path: Path) -> None: + """A real failure *inside* the clone (bad ref, missing commit) must still fail.""" + stderr = "copier._vcs.clone.ab12cd34: fatal: invalid reference: HEAD" + assert not _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) + + def test_rejects_when_nothing_was_rendered(self, tmp_path: Path) -> None: + """The load-bearing gate: cleanup noise never excuses an empty destination.""" + assert not _is_temp_clone_cleanup_failure(self.CLONE_CLEANUP_STDERR, tmp_path) From b2cf46455387674819b957f943cfe27b395d63e8 Mon Sep 17 00:00:00 2001 From: Ismar <1242091+ichoosetoaccept@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:44:34 +0200 Subject: [PATCH 3/4] fix(test): bind the cleanup tolerance to the failure that raised it The matcher looked for three independent substrings anywhere in stderr: `copier._vcs.clone`, one of `Directory not empty`/`rmtree`/`Errno 66`, and a rendered `pyproject.toml` in the destination. Every one of those can be true of a *failed* render. Copier renders from the temp clone, so every template frame in a Jinja traceback carries a `copier._vcs.clone.*` path -- the first check is close to vacuous. Worse, the cleanup crash is raised while unwinding, so a real render error and the `OSError` appear together in the same stderr, and `pyproject.toml` is written early enough to survive a later failure. The suite would have run against a half-rendered project and reported green. Now all four must hold: the final `OSError` line itself names a path under copier's temp clone, it was raised from a `_cleanup` frame, no chained-exception marker precedes it, and the destination received a render. Chaining is the discriminator that matters -- it is precisely how a partial render presents. Verified against the real thing: reproduced the dirty-tree failure (copier 9.17.1, macOS), captured stderr, and confirmed the matcher accepts it. Against the two failure shapes, the old matcher returned True for both and the new one returns False for both: CLEANUP_ONLY old=True new=True PARTIAL_RENDER_THEN_CLEANUP old=True new=False RENDER_ERROR_ONLY old=True new=False Tests rewritten around trimmed copies of real tracebacks rather than synthetic marker strings, which is what let the loose matcher look covered. Fast suite: 208 passed. Reported by Greptile on #348. --- tests/conftest.py | 49 ++++++++++++++++++++++++++---- tests/test_template.py | 69 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1f30e32..ac84396 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ from __future__ import annotations +import re import subprocess from pathlib import Path @@ -9,6 +10,26 @@ REPO_ROOT = Path(__file__).parent.parent +# The one copier failure the suite tolerates (DOT-606), matched as a whole rather than as +# loose substrings. The `OSError` line must itself name a path under copier's own temp +# clone: a render error's traceback also mentions that directory — every template frame +# lives inside it — so `"copier._vcs.clone" in stderr` is nearly vacuous on its own. +_CLONE_CLEANUP_RE = re.compile( + r"^OSError: \[Errno \d+\] Directory not empty: .*copier\._vcs\.clone\.", + re.MULTILINE, +) + +# ...and it must have been raised *by* the cleanup, not merely alongside it. +_CLEANUP_FRAME = ", in _cleanup" + +# A chained traceback means something failed first and the cleanup crash rode along on the +# unwind. That is the dangerous shape: a partial render whose exception is followed by the +# cleanup `OSError`, which would otherwise satisfy every check above. +_CHAINED_EXCEPTION_MARKERS = ( + "During handling of the above exception", + "The above exception was the direct cause", +) + @pytest.fixture def copier_defaults() -> dict: @@ -89,14 +110,30 @@ def _is_temp_clone_cleanup_failure(stderr: str, dst: Path) -> bool: first, so it reads as flakiness rather than as one deterministic bug. Re-running the "failed" test alone usually passes, which sends triage down the wrong path entirely. - Deliberately narrow. This tolerates only an `rmtree` of a path copier itself names - `copier._vcs.clone.*`, and only when the destination actually received a rendered - project. A render that genuinely failed leaves no `pyproject.toml`, and any other - non-zero exit still fails the test with copier's own stderr. + Deliberately narrow, because this is the one place a real copier failure could be + swallowed. All four must hold: + + 1. The final exception is an `OSError` naming a path under copier's own temp clone. + 2. It was raised from a `_cleanup` frame — the cleanup is what failed, not something + that merely happened to mention the same directory. + 3. Nothing failed before it. A chained traceback is exactly how a partial render would + present: its own exception, then the cleanup `OSError` raised while unwinding. + 4. The destination actually received a render. + + Independent substring checks are not enough for (1) and (2): every template frame in a + Jinja render error lives *inside* the temp clone, so a genuine render failure can put + `copier._vcs.clone` and `rmtree` in stderr on unrelated lines while `pyproject.toml` — + written early — already exists in the destination. Any other non-zero exit still fails + the test with copier's own stderr. + + If copier renames `_cleanup`, this stops matching and DOT-606's dirty-tree failures + come back visibly. That is the intended direction to fail in. """ - if "copier._vcs.clone" not in stderr: + if not _CLONE_CLEANUP_RE.search(stderr): + return False + if _CLEANUP_FRAME not in stderr: return False - if not any(marker in stderr for marker in ("Directory not empty", "rmtree", "Errno 66")): + if any(marker in stderr for marker in _CHAINED_EXCEPTION_MARKERS): return False return (dst / "pyproject.toml").is_file() diff --git a/tests/test_template.py b/tests/test_template.py index 15e0b4f..6d5feef 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -2665,7 +2665,39 @@ class TestTempCloneCleanupTolerance: this helper is never consulted at all. """ - CLONE_CLEANUP_STDERR = "OSError: [Errno 66] Directory not empty: '/tmp/copier._vcs.clone.ab12cd34/.git'" + # Trimmed from a real failure: dirty tree, `copier copy -r HEAD`, copier 9.17.1 on + # macOS. Frame paths shortened, structure and the final line kept verbatim. + CLEANUP_ONLY = """Traceback (most recent call last): + File "/venv/copier/_template.py", line 240, in _cleanup + rmtree(temp_clone, ignore_errors=False, onexc=handle_remove_readonly) + File "/python/shutil.py", line 753, in _rmtree_safe_fd_step + os.rmdir(name, dir_fd=dirfd) +OSError: [Errno 66] Directory not empty: PosixPath('/tmp/copier._vcs.clone.ulvgwzp7') +""" + + # A render that died partway, with the cleanup crash chained onto the unwind. Every + # loose substring the old matcher looked for is present, and `pyproject.toml` was + # written before the failure — so only the chaining marker separates this from benign. + PARTIAL_RENDER_THEN_CLEANUP = """Traceback (most recent call last): + File "/venv/copier/_main.py", line 850, in _render_file + raise UndefinedError(msg) +jinja2.exceptions.UndefinedError: 'project_name' is undefined + +During handling of the above exception, another exception occurred: + +Traceback (most recent call last): + File "/venv/copier/_template.py", line 240, in _cleanup + rmtree(temp_clone, ignore_errors=False, onexc=handle_remove_readonly) +OSError: [Errno 66] Directory not empty: PosixPath('/tmp/copier._vcs.clone.ulvgwzp7') +""" + + # A pure render failure. Mentions the clone directory (every template frame lives + # inside it) and the word `rmtree`, but nothing here is a cleanup failure. + RENDER_ERROR_ONLY = """Traceback (most recent call last): + File "/tmp/copier._vcs.clone.ulvgwzp7/project/scripts/rmtree_helper.py", line 3 + {{ project_name } +jinja2.exceptions.TemplateSyntaxError: unexpected '}' +""" @staticmethod def _with_render(tmp_path: Path) -> Path: @@ -2673,22 +2705,33 @@ def _with_render(tmp_path: Path) -> Path: (tmp_path / "pyproject.toml").write_text('[project]\nname = "rendered"\n') return tmp_path - @pytest.mark.parametrize("marker", ["Directory not empty", "rmtree", "Errno 66"]) - def test_accepts_each_cleanup_marker(self, tmp_path: Path, marker: str) -> None: - """Each accepted signature is tested alone — no case leans on another's marker.""" - stderr = f"copier._vcs.clone.ab12cd34 cleanup failed: {marker}" - assert _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) + def test_accepts_a_cleanup_only_failure(self, tmp_path: Path) -> None: + """The one shape this exists for: render finished, `_cleanup` could not rmtree.""" + assert _is_temp_clone_cleanup_failure(self.CLEANUP_ONLY, self._with_render(tmp_path)) - def test_rejects_failure_outside_copiers_temp_clone(self, tmp_path: Path) -> None: - """Same OSError, a path copier did not create — could be the template's own tree.""" - stderr = "OSError: [Errno 66] Directory not empty: '/some/other/path/.git'" + def test_rejects_partial_render_with_chained_cleanup_failure(self, tmp_path: Path) -> None: + """The dangerous case: a real render error whose unwind also trips the cleanup. + + Every individual token the matcher keys on is present, and the destination holds a + `pyproject.toml` written before the failure. Suppressing this would run the whole + suite against a half-rendered project. + """ + assert not _is_temp_clone_cleanup_failure(self.PARTIAL_RENDER_THEN_CLEANUP, self._with_render(tmp_path)) + + def test_rejects_render_error_that_merely_mentions_the_clone(self, tmp_path: Path) -> None: + """`copier._vcs.clone` and `rmtree` in stderr prove nothing on their own.""" + assert not _is_temp_clone_cleanup_failure(self.RENDER_ERROR_ONLY, self._with_render(tmp_path)) + + def test_rejects_cleanup_failure_outside_copiers_temp_clone(self, tmp_path: Path) -> None: + """Same exception, a directory copier did not create — not ours to excuse.""" + stderr = self.CLEANUP_ONLY.replace("copier._vcs.clone.ulvgwzp7", "some-other-dir") assert not _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) - def test_rejects_clone_path_without_a_cleanup_marker(self, tmp_path: Path) -> None: - """A real failure *inside* the clone (bad ref, missing commit) must still fail.""" - stderr = "copier._vcs.clone.ab12cd34: fatal: invalid reference: HEAD" + def test_rejects_when_the_failure_was_not_raised_by_cleanup(self, tmp_path: Path) -> None: + """Bind the OSError to the `_cleanup` frame, not just to the clone path.""" + stderr = self.CLEANUP_ONLY.replace(", in _cleanup", ", in _render_file") assert not _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) def test_rejects_when_nothing_was_rendered(self, tmp_path: Path) -> None: """The load-bearing gate: cleanup noise never excuses an empty destination.""" - assert not _is_temp_clone_cleanup_failure(self.CLONE_CLEANUP_STDERR, tmp_path) + assert not _is_temp_clone_cleanup_failure(self.CLEANUP_ONLY, tmp_path) From 138e14aa0f8f2947837dc8e750385dd3211fae03 Mon Sep 17 00:00:00 2001 From: Ismar <1242091+ichoosetoaccept@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:57:51 +0200 Subject: [PATCH 4/4] fix(test): retry the cleanup race instead of tolerating a failed exit Greptile's follow-up is right that no stderr pattern can prove a render completed. `raise ... from None` would drop the chaining marker the previous commit leaned on, and in general the absence of evidence of an earlier failure is not evidence of its absence. Copier itself has no `from None` or `__suppress_context__` anywhere in 9.17.1, so the specific mechanism cannot fire today -- but the objection stands on principle, and the design that answers it is simpler than the matcher it replaces. So stop accepting non-zero exits at all. The race is intermittent -- the same dirty tree renders fine on the next attempt -- so a re-render is the actual remedy, and stderr matching now only decides whether to spend one. Three attempts against a hit rate around one render in ten; lose three in a row and the test fails with copier's stderr, which is the safe direction. The matcher stays tight to keep pointless retries rare, but it is no longer load-bearing: a false positive costs one render and then fails anyway, a false negative fails immediately, and neither can hide a partial render. Renamed to `_is_retryable_cleanup_race` to stop implying it certifies success. Verified a genuine copier failure (a missing required answer) aborts on attempt 1/3 with copier's full traceback, no retry. Fast suite on a dirty tree: 208 passed. Reported by Greptile on #348. --- tests/conftest.py | 42 +++++++++++++++++++++++++++++------------- tests/test_template.py | 32 +++++++++++++++++--------------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ac84396..4ae00f5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -30,6 +30,12 @@ "The above exception was the direct cause", ) +# The race is intermittent — the same dirty tree renders fine on the next attempt — so a +# re-render is the actual fix and stderr matching only decides whether to spend one. Three +# is chosen against a measured hit rate around one render in ten; a run that loses the race +# three times running fails, which is the safe direction. +_MAX_RENDER_ATTEMPTS = 3 + @pytest.fixture def copier_defaults() -> dict: @@ -90,15 +96,21 @@ def generate_project( else: cmd.extend(["-d", f"{key}={value}"]) - result = subprocess.run(cmd, capture_output=True, text=True, check=False) - if result.returncode != 0 and not _is_temp_clone_cleanup_failure(result.stderr, tmp_path): - pytest.fail(f"Copier failed: {result.stderr}") + for attempt in range(1, _MAX_RENDER_ATTEMPTS + 1): + result = subprocess.run(cmd, capture_output=True, text=True, check=False) + if result.returncode == 0: + return tmp_path + # A non-zero exit is never accepted as success. The cleanup race only buys another + # attempt; if copier keeps failing, the last stderr is what the test reports. + if attempt < _MAX_RENDER_ATTEMPTS and _is_retryable_cleanup_race(result.stderr, tmp_path): + continue + pytest.fail(f"Copier failed (attempt {attempt}/{_MAX_RENDER_ATTEMPTS}): {result.stderr}") return tmp_path -def _is_temp_clone_cleanup_failure(stderr: str, dst: Path) -> bool: - """True if copier rendered successfully but crashed clearing its own temp clone (DOT-606). +def _is_retryable_cleanup_race(stderr: str, dst: Path) -> bool: + """True if copier's exit looks like the temp-clone cleanup race, so re-rendering is worth a shot (DOT-606). On a dirty working tree copier takes its dirty-file overlay path, which does extra git work inside the temp clone it made of this repo. Something still holds a handle under @@ -110,24 +122,28 @@ def _is_temp_clone_cleanup_failure(stderr: str, dst: Path) -> bool: first, so it reads as flakiness rather than as one deterministic bug. Re-running the "failed" test alone usually passes, which sends triage down the wrong path entirely. - Deliberately narrow, because this is the one place a real copier failure could be - swallowed. All four must hold: + This decides whether to *retry*, never whether to accept a failure. That distinction is + what bounds the damage of a wrong answer: a false positive costs one extra render and + then fails with copier's real stderr anyway, and a false negative fails immediately — + which is what would have happened without any of this. Nothing is ever suppressed, so + no amount of cleverness in classifying stderr is load-bearing for correctness. + + Still matched tightly, to keep pointless retries rare. All four must hold: 1. The final exception is an `OSError` naming a path under copier's own temp clone. 2. It was raised from a `_cleanup` frame — the cleanup is what failed, not something that merely happened to mention the same directory. - 3. Nothing failed before it. A chained traceback is exactly how a partial render would - present: its own exception, then the cleanup `OSError` raised while unwinding. + 3. Nothing failed before it. A chained traceback is how a partial render presents: its + own exception, then the cleanup `OSError` raised while unwinding. 4. The destination actually received a render. Independent substring checks are not enough for (1) and (2): every template frame in a Jinja render error lives *inside* the temp clone, so a genuine render failure can put `copier._vcs.clone` and `rmtree` in stderr on unrelated lines while `pyproject.toml` — - written early — already exists in the destination. Any other non-zero exit still fails - the test with copier's own stderr. + written early — already exists in the destination. - If copier renames `_cleanup`, this stops matching and DOT-606's dirty-tree failures - come back visibly. That is the intended direction to fail in. + If copier renames `_cleanup`, this stops matching and the race stops being retried. + That is the intended direction to fail in. """ if not _CLONE_CLEANUP_RE.search(stderr): return False diff --git a/tests/test_template.py b/tests/test_template.py index 6d5feef..7f163a8 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -13,7 +13,7 @@ import pytest import yaml -from conftest import REPO_ROOT, _is_temp_clone_cleanup_failure, generate_project +from conftest import REPO_ROOT, _is_retryable_cleanup_race, generate_project if TYPE_CHECKING: from pathlib import Path @@ -2655,14 +2655,16 @@ def test_semantic_release_writes_gitlab_urls_for_selfhosted(self, tmp_path: Path class TestTempCloneCleanupTolerance: - """Pin the boundary of `conftest._is_temp_clone_cleanup_failure` (DOT-606). - - That helper is the one place in the suite where a non-zero copier exit is suppressed, - and both directions of drift are silent. Widen it and a genuine render failure passes - as a green test; narrow it and the dirty-tree failures it exists to absorb come back. - Neither shows up in the integration tests, which only ever see whichever exit copier - happens to produce on the machine running them — on a clean tree, that is exit 0 and - this helper is never consulted at all. + """Pin the boundary of `conftest._is_retryable_cleanup_race` (DOT-606). + + The helper decides whether a failed render is worth re-running, so a wrong answer costs + a wasted render or an early failure — never a swallowed one. It is still worth pinning: + widen it and the suite burns three renders on every genuinely broken template; narrow it + and DOT-606's dirty-tree failures stop being retried and come back. + + The integration tests cannot cover this. They only see whichever exit copier produces on + the machine running them, and on a clean tree that is exit 0 with the helper never + consulted at all. """ # Trimmed from a real failure: dirty tree, `copier copy -r HEAD`, copier 9.17.1 on @@ -2707,7 +2709,7 @@ def _with_render(tmp_path: Path) -> Path: def test_accepts_a_cleanup_only_failure(self, tmp_path: Path) -> None: """The one shape this exists for: render finished, `_cleanup` could not rmtree.""" - assert _is_temp_clone_cleanup_failure(self.CLEANUP_ONLY, self._with_render(tmp_path)) + assert _is_retryable_cleanup_race(self.CLEANUP_ONLY, self._with_render(tmp_path)) def test_rejects_partial_render_with_chained_cleanup_failure(self, tmp_path: Path) -> None: """The dangerous case: a real render error whose unwind also trips the cleanup. @@ -2716,22 +2718,22 @@ def test_rejects_partial_render_with_chained_cleanup_failure(self, tmp_path: Pat `pyproject.toml` written before the failure. Suppressing this would run the whole suite against a half-rendered project. """ - assert not _is_temp_clone_cleanup_failure(self.PARTIAL_RENDER_THEN_CLEANUP, self._with_render(tmp_path)) + assert not _is_retryable_cleanup_race(self.PARTIAL_RENDER_THEN_CLEANUP, self._with_render(tmp_path)) def test_rejects_render_error_that_merely_mentions_the_clone(self, tmp_path: Path) -> None: """`copier._vcs.clone` and `rmtree` in stderr prove nothing on their own.""" - assert not _is_temp_clone_cleanup_failure(self.RENDER_ERROR_ONLY, self._with_render(tmp_path)) + assert not _is_retryable_cleanup_race(self.RENDER_ERROR_ONLY, self._with_render(tmp_path)) def test_rejects_cleanup_failure_outside_copiers_temp_clone(self, tmp_path: Path) -> None: """Same exception, a directory copier did not create — not ours to excuse.""" stderr = self.CLEANUP_ONLY.replace("copier._vcs.clone.ulvgwzp7", "some-other-dir") - assert not _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) + assert not _is_retryable_cleanup_race(stderr, self._with_render(tmp_path)) def test_rejects_when_the_failure_was_not_raised_by_cleanup(self, tmp_path: Path) -> None: """Bind the OSError to the `_cleanup` frame, not just to the clone path.""" stderr = self.CLEANUP_ONLY.replace(", in _cleanup", ", in _render_file") - assert not _is_temp_clone_cleanup_failure(stderr, self._with_render(tmp_path)) + assert not _is_retryable_cleanup_race(stderr, self._with_render(tmp_path)) def test_rejects_when_nothing_was_rendered(self, tmp_path: Path) -> None: """The load-bearing gate: cleanup noise never excuses an empty destination.""" - assert not _is_temp_clone_cleanup_failure(self.CLEANUP_ONLY, tmp_path) + assert not _is_retryable_cleanup_race(self.CLEANUP_ONLY, tmp_path)