test: tolerate copier's temp-clone cleanup race on a dirty tree

tests/conftest.py

@@ -2,13 +2,40 @@
 
 from __future__ import annotations
 
+import re
 import subprocess
 from pathlib import Path
 
 import pytest
 
 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",
+)
+
+# 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:
@@ -69,13 +96,64 @@ 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:
-        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_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
+    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.
+
+    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 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.
+
+    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
+    if _CLEANUP_FRAME not in stderr:
+        return False
+    if any(marker in stderr for marker in _CHAINED_EXCEPTION_MARKERS):
+        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}

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_retryable_cleanup_race, generate_project
 
 if TYPE_CHECKING:
     from pathlib import Path
@@ -2652,3 +2652,88 @@ 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_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
+    # 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:
+        """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
+
+    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_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.
+
+        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_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_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_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_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_retryable_cleanup_race(self.CLEANUP_ONLY, tmp_path)