Fix tmp path retention 14998

AUTHORS

@@ -383,6 +383,7 @@ Omri Golan
 Ondřej Súkup
 Onkesh Bansal
 Oscar Benjamin
+Panayiotis Michalos
 Paneendra Kumar
 Parth Patel
 Patrick Hayes

changelog/14998.bugfix.rst

@@ -0,0 +1,2 @@
+Fixed ``tmp_path_retention_policy="failed"`` incorrectly removing temporary
+directories when fixture setup or teardown raises an error.

src/_pytest/tmpdir.py

@@ -37,6 +37,7 @@
 
 
 tmppath_result_key = StashKey[dict[str, bool]]()
+tmppath_path_key = StashKey[Path]()
 RetentionType = Literal["all", "failed", "none"]
 
 
@@ -292,18 +293,11 @@ def tmp_path(
     as discussed in :ref:`temporary directory location and retention`.
     """
     path = _mk_tmp(request, tmp_path_factory)
-    yield path
-
-    # Remove the tmpdir if the policy is "failed" and the test passed.
-    policy = tmp_path_factory._retention_policy
-    result_dict = request.node.stash[tmppath_result_key]
 
-    if policy == "failed" and result_dict.get("call", True):
-        # We do a "best effort" to remove files, but it might not be possible due to some leaked resource,
-        # permissions, etc, in which case we ignore it.
-        rmtree(path, ignore_errors=True)
+    # Store the path so cleanup can happen after the teardown report.
+    request.node.stash[tmppath_path_key] = path
 
-    del request.node.stash[tmppath_result_key]
+    yield path
 
 
 def pytest_sessionfinish(session, exitstatus: int | ExitCode):
@@ -340,6 +334,25 @@ def pytest_runtest_makereport(
 ) -> Generator[None, TestReport, TestReport]:
     rep = yield
     assert rep.when is not None
-    empty: dict[str, bool] = {}
-    item.stash.setdefault(tmppath_result_key, empty)[rep.when] = rep.passed
+
+    # Record whether each phase failed.
+    result_dict = item.stash.setdefault(tmppath_result_key, {})
+    result_dict[rep.when] = rep.failed
+
+    # Wait until teardown has finished before deciding whether the temporary directory should be retained.
+    if rep.when == "teardown":
+        path = item.stash.get(tmppath_path_key, None)
+
+        if path is not None:
+            tmp_path_factory: TempPathFactory = item.config._tmp_path_factory  # type: ignore[attr-defined]
+            policy = tmp_path_factory._retention_policy
+
+            # Retain the directory if any phase failed. Otherwise, remove it under the "failed" policy.
+            if policy == "failed" and not any(result_dict.values()):
+                rmtree(path, ignore_errors=True)
+
+            del item.stash[tmppath_path_key]
+
+        del item.stash[tmppath_result_key]
+
     return rep

testing/test_tmpdir.py

@@ -197,6 +197,53 @@ def test_2(tmp_path):
             assert len(test_dir) == 1
             assert test_dir[0].name == "test_20"
 
+    def test_policy_failed_keeps_setup_and_teardown_errors(
+        self, pytester: Pytester
+    ) -> None:
+        """Regression test for #14998."""
+        pytester.makepyprojecttoml(
+            """
+            [tool.pytest.ini_options]
+            tmp_path_retention_policy = "failed"
+            """
+        )
+
+        pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture
+            def setup_error(tmp_path):
+                (tmp_path / "evidence.txt").write_text("setup", encoding='utf-8')
+                raise RuntimeError("setup failed")
+
+            @pytest.fixture
+            def teardown_error(tmp_path):
+                (tmp_path / "evidence.txt").write_text("teardown", encoding='utf-8')
+                yield
+                raise RuntimeError("teardown failed")
+
+            def test_setup(setup_error):
+                pass
+
+            def test_call(tmp_path):
+                (tmp_path / "evidence.txt").write_text("call", encoding='utf-8')
+                assert False
+
+            def test_teardown(teardown_error):
+                pass
+            """
+        )
+
+        result = pytester.runpytest("--basetemp=retained")
+        result.assert_outcomes(passed=1, failed=1, errors=2)
+
+        retained = pytester.path / "retained"
+
+        assert (retained / "test_setup0" / "evidence.txt").exists()
+        assert (retained / "test_call0" / "evidence.txt").exists()
+        assert (retained / "test_teardown0" / "evidence.txt").exists()
+
     def test_policy_failed_removes_basedir_when_all_passed(
         self, pytester: Pytester
     ) -> None: