From 99dfc19fe62c15cd8717d30a3d5b50352931697a Mon Sep 17 00:00:00 2001
From: Yusuke Kadowaki <60080334+yusuke-kadowaki@users.noreply.github.com>
Date: Wed, 23 Nov 2022 22:48:29 +0900
Subject: [PATCH] Fix `tmp_path_retention_policy` crash when skipping from
 fixture (#10517)

Also uses the stash to save the test status.

Fix #10502
---
 src/_pytest/tmpdir.py  | 20 +++++++++++--
 testing/test_tmpdir.py | 64 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py
index 7b8e42017..3fd8168b6 100644
--- a/src/_pytest/tmpdir.py
+++ b/src/_pytest/tmpdir.py
@@ -5,11 +5,15 @@ import sys
 import tempfile
 from pathlib import Path
 from shutil import rmtree
+from typing import Dict
 from typing import Generator
 from typing import Optional
 from typing import TYPE_CHECKING
 from typing import Union
 
+from _pytest.nodes import Item
+from _pytest.stash import StashKey
+
 if TYPE_CHECKING:
     from typing_extensions import Literal
 
@@ -33,6 +37,8 @@ from _pytest.fixtures import fixture
 from _pytest.fixtures import FixtureRequest
 from _pytest.monkeypatch import MonkeyPatch
 
+tmppath_result_key = StashKey[Dict[str, bool]]()
+
 
 @final
 @attr.s(init=False)
@@ -273,11 +279,15 @@ def tmp_path(
     # Remove the tmpdir if the policy is "failed" and the test passed.
     tmp_path_factory: TempPathFactory = request.session.config._tmp_path_factory  # type: ignore
     policy = tmp_path_factory._retention_policy
-    if policy == "failed" and request.node._tmp_path_result_call.passed:
+    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)
 
+    del request.node.stash[tmppath_result_key]
+
     # remove dead symlink
     basetemp = tmp_path_factory._basetemp
     if basetemp is None:
@@ -306,7 +316,11 @@ def pytest_sessionfinish(session, exitstatus: Union[int, ExitCode]):
 
 
 @hookimpl(tryfirst=True, hookwrapper=True)
-def pytest_runtest_makereport(item, call):
+def pytest_runtest_makereport(item: Item, call):
     outcome = yield
     result = outcome.get_result()
-    setattr(item, "_tmp_path_result_" + result.when, result)
+
+    if tmppath_result_key not in item.stash:
+        item.stash[tmppath_result_key] = {result.when: result.passed}
+    else:
+        item.stash[tmppath_result_key][result.when] = result.passed
diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py
index e681470a4..57f442b04 100644
--- a/testing/test_tmpdir.py
+++ b/testing/test_tmpdir.py
@@ -139,6 +139,70 @@ class TestConfigTmpPath:
             # Check the base dir itself is gone
             assert len(list(base_dir)) == 0
 
+    # issue #10502
+    def test_policy_failed_removes_dir_when_skipped_from_fixture(
+        self, pytester: Pytester
+    ) -> None:
+        p = pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture
+            def fixt(tmp_path):
+                pytest.skip()
+
+            def test_fixt(fixt):
+                pass
+        """
+        )
+        pytester.inline_run(p)
+
+        # Check if the whole directory is removed
+        root = pytester._test_tmproot
+        for child in root.iterdir():
+            base_dir = list(
+                filter(lambda x: x.is_dir() and not x.is_symlink(), child.iterdir())
+            )
+            assert len(base_dir) == 0
+
+    # issue #10502
+    def test_policy_all_keeps_dir_when_skipped_from_fixture(
+        self, pytester: Pytester
+    ) -> None:
+        p = pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture
+            def fixt(tmp_path):
+                pytest.skip()
+
+            def test_fixt(fixt):
+                pass
+        """
+        )
+        pytester.makepyprojecttoml(
+            """
+            [tool.pytest.ini_options]
+            tmp_path_retention_policy = "all"
+        """
+        )
+        pytester.inline_run(p)
+
+        # Check if the whole directory is kept
+        root = pytester._test_tmproot
+        for child in root.iterdir():
+            base_dir = list(
+                filter(lambda x: x.is_dir() and not x.is_symlink(), child.iterdir())
+            )
+            assert len(base_dir) == 1
+            test_dir = list(
+                filter(
+                    lambda x: x.is_dir() and not x.is_symlink(), base_dir[0].iterdir()
+                )
+            )
+            assert len(test_dir) == 1
+
 
 testdata = [
     ("mypath", True),