From d54c3ac94517824547c3275756398429bcde7681 Mon Sep 17 00:00:00 2001 From: Dextheking1 Date: Wed, 23 Sep 2026 18:59:39 +0200 Subject: [PATCH] Fix tmp_path_retention_policy=failed deleting dirs on setup/teardown errors With tmp_path_retention_policy=failed, tmp_path directories were removed when an error occurred during fixture setup or teardown: setup errors fell back to the default deletion because no call report existed, and teardown errors were missed because the fixture finalizer ran before the teardown report was produced. Defer the retention decision to the teardown report: the tmp_path fixture now stashes its directory and the makereport hook removes it only once setup, call and teardown all passed. Outcomes (not just passed flags) are recorded so a skip during fixture setup still removes the directory. Fixes #14998 --- changelog/14998.bugfix.rst | 4 +++ src/_pytest/tmpdir.py | 48 +++++++++++++++++++++++++-------- testing/test_tmpdir.py | 55 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 changelog/14998.bugfix.rst diff --git a/changelog/14998.bugfix.rst b/changelog/14998.bugfix.rst new file mode 100644 index 00000000000..c791a854207 --- /dev/null +++ b/changelog/14998.bugfix.rst @@ -0,0 +1,4 @@ +Fixed ``tmp_path_retention_policy="failed"`` removing ``tmp_path`` directories when an +error occurred during fixture setup or teardown. The directories are now retained +for setup and teardown errors, just like for call-phase failures. A skip during +fixture setup (without an error) still removes the directory. diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 745a3c95670..9d4d02fdded 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -36,7 +36,8 @@ from _pytest.stash import StashKey -tmppath_result_key = StashKey[dict[str, bool]]() +tmppath_result_key = StashKey[dict[str, str]]() +tmppath_pending_key = StashKey[list[Path]]() RetentionType = Literal["all", "failed", "none"] @@ -295,16 +296,35 @@ def tmp_path( 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] + # Defer the retention decision until the teardown report is available: + # the teardown outcome is not known yet at this point, so a teardown + # error could not be taken into account here. See + # pytest_runtest_makereport below. + request.node.stash.setdefault(tmppath_pending_key, []).append(path) + - 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) +def _maybe_remove_tmp_paths(item: Item, results: dict[str, str]) -> None: + """Remove pending tmp_path directories if the test fully passed. - del request.node.stash[tmppath_result_key] + Called once the teardown report is available, so setup, call and + teardown outcomes can all be taken into account. + """ + policy: RetentionType = item.config._tmp_path_factory._retention_policy + paths = item.stash.get(tmppath_pending_key, []) + # Only remove the directory when the test fully passed: any failure or + # error in setup, call or teardown keeps the directory around. + # A skip during setup (without an error) still removes it (#10502). + if ( + policy == "failed" + and paths + and results.get("setup", "passed") in ("passed", "skipped") + and results.get("call", "passed") == "passed" + and results.get("teardown", "passed") in ("passed", "skipped") + ): + for path in paths: + # 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) def pytest_sessionfinish(session, exitstatus: int | ExitCode): @@ -341,6 +361,12 @@ 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 + empty: dict[str, str] = {} + results = item.stash.setdefault(tmppath_result_key, empty) + results[rep.when] = rep.outcome + if rep.when == "teardown": + _maybe_remove_tmp_paths(item, results) + del item.stash[tmppath_result_key] + if tmppath_pending_key in item.stash: + del item.stash[tmppath_pending_key] return rep diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 0b33a74b926..c243dda0318 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -292,6 +292,61 @@ def test_fixt(fixt): ) assert len(test_dir) == 1 + # issue #14998 + def test_policy_failed_keeps_dir_on_setup_and_teardown_errors( + self, pytester: Pytester + ) -> None: + p = pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def broken_during_setup(tmp_path): + raise RuntimeError("setup failed") + + def test_setup_error(broken_during_setup): + pass + + def test_call_failure(tmp_path): + assert False + + @pytest.fixture + def broken_during_teardown(tmp_path): + yield + raise RuntimeError("teardown failed") + + def test_teardown_error(broken_during_teardown): + pass + """ + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + tmp_path_retention_policy = "failed" + """ + ) + + result = pytester.inline_run(p) + # 1 failed (call), 1 passed (call of the teardown-error test), + # 3 failed reports overall (setup error, call failure, teardown error). + result.assertoutcome(passed=1, failed=3) + + root = pytester._test_tmproot + kept = set() + for child in root.iterdir(): + for base_dir in child.iterdir(): + if base_dir.is_dir() and not base_dir.is_symlink(): + for test_dir in base_dir.iterdir(): + if test_dir.is_dir() and not test_dir.is_symlink(): + kept.add(test_dir.name) + # The directories for the setup and teardown errors must be retained, + # just like the one for the call failure. + assert kept == { + "test_setup_error0", + "test_call_failure0", + "test_teardown_error0", + } + testdata = [ ("mypath", True),