diff --git a/AUTHORS b/AUTHORS index e2fad5e8364..b264089180b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -406,6 +406,7 @@ Pulkit Goyal Punyashloka Biswal Quentin Pradet q0w +qinpeili Ralf Schmitt Ralph Giles Ram Rachum diff --git a/changelog/14998.bugfix.rst b/changelog/14998.bugfix.rst new file mode 100644 index 00000000000..6a7352112e8 --- /dev/null +++ b/changelog/14998.bugfix.rst @@ -0,0 +1 @@ +Fixed :confval:`tmp_path_retention_policy` ``failed`` to retain ``tmp_path`` directories when tests fail during setup or teardown. diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 745a3c95670..45a7298b4ab 100644 --- a/src/_pytest/tmpdir.py +++ b/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"] @@ -293,19 +294,9 @@ def tmp_path( as discussed in :ref:`temporary directory location and retention`. """ path = _mk_tmp(request, tmp_path_factory) + request.node.stash[tmppath_path_key] = path 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) - - del request.node.stash[tmppath_result_key] - def pytest_sessionfinish(session, exitstatus: int | ExitCode): """After each session, remove base directory if all the tests passed, @@ -342,5 +333,17 @@ def pytest_runtest_makereport( rep = yield assert rep.when is not None empty: dict[str, bool] = {} - item.stash.setdefault(tmppath_result_key, empty)[rep.when] = rep.passed + result_dict = item.stash.setdefault(tmppath_result_key, empty) + result_dict[rep.when] = rep.failed + if rep.when == "teardown": + # The teardown report is the first point where all phase outcomes are known. + path = item.stash.get(tmppath_path_key, None) + if path is not None: + policy = item.config._tmp_path_factory._retention_policy # type: ignore[attr-defined] + if policy == "failed" and not any(result_dict.values()): + # 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 item.stash[tmppath_path_key] + del item.stash[tmppath_result_key] return rep diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 0b33a74b926..16a39631423 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -223,6 +223,87 @@ def test_1(tmp_path): # Check the base dir itself is gone assert len(list(base_dir)) == 0 + @pytest.mark.parametrize( + ("test_source", "outcomes", "kept"), + [ + ( + """ + import pytest + + @pytest.fixture + def broken_setup(tmp_path): + raise RuntimeError("setup error") + + def test_setup_error(broken_setup): + pass + """, + {"errors": 1}, + True, + ), + ( + """ + def test_call_failure(tmp_path): + assert False + """, + {"failed": 1}, + True, + ), + ( + """ + import pytest + + @pytest.fixture + def broken_teardown(tmp_path): + yield + raise RuntimeError("teardown error") + + def test_teardown_error(broken_teardown): + pass + """, + {"passed": 1, "errors": 1}, + True, + ), + ( + """ + import pytest + + @pytest.fixture + def skipped_setup(tmp_path): + pytest.skip("setup skip") + + def test_setup_skip(skipped_setup): + pass + """, + {"skipped": 1}, + False, + ), + ( + """ + def test_pass(tmp_path): + pass + """, + {"passed": 1}, + False, + ), + ], + ids=["setup-error", "call-failure", "teardown-error", "setup-skip", "pass"], + ) + def test_policy_failed_retains_dirs_for_errors( + self, pytester: Pytester, test_source: str, outcomes: dict[str, int], kept: bool + ) -> None: + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + tmp_path_retention_policy = "failed" + """ + ) + pytester.makepyfile(test_source) + result = pytester.runpytest() + result.assert_outcomes(**outcomes) + + retained = list((pytester.path.parent / "basetemp").glob("test_*")) + assert bool(retained) is kept + # issue #10502 def test_policy_failed_removes_dir_when_skipped_from_fixture( self, pytester: Pytester