diff --git a/AUTHORS b/AUTHORS index daa6a471eb9..0ded3545655 100644 --- a/AUTHORS +++ b/AUTHORS @@ -202,6 +202,7 @@ Greg Price Gregory Lee Grig Gheorghiu Grigorii Eremeev (budulianin) +GruffElixir Guido Wesdorp Guoqiang Zhang Hamza Mobeen diff --git a/changelog/14998.bugfix.rst b/changelog/14998.bugfix.rst new file mode 100644 index 00000000000..a1f06bb57ac --- /dev/null +++ b/changelog/14998.bugfix.rst @@ -0,0 +1 @@ +The ``tmp_path_retention_policy = "failed"`` setting now retains temporary directories when a test errors during fixture setup or teardown. diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 745a3c95670..ecfee45553d 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -37,6 +37,8 @@ tmppath_result_key = StashKey[dict[str, bool]]() +tmppath_path_key = StashKey[Path]() +tmppath_setup_skipped_key = StashKey[bool]() RetentionType = Literal["all", "failed", "none"] @@ -293,18 +295,15 @@ def tmp_path( as discussed in :ref:`temporary directory location and retention`. """ path = _mk_tmp(request, tmp_path_factory) + if tmp_path_factory._retention_policy == "failed": + 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] + # The retention decision is made in pytest_runtest_makereport after the + # teardown report is available. This ensures setup and teardown errors are + # retained under the "failed" policy too. + if tmp_path_factory._retention_policy != "failed": + del request.node.stash[tmppath_result_key] def pytest_sessionfinish(session, exitstatus: int | ExitCode): @@ -343,4 +342,29 @@ def pytest_runtest_makereport( assert rep.when is not None empty: dict[str, bool] = {} item.stash.setdefault(tmppath_result_key, empty)[rep.when] = rep.passed + + tmp_path = item.stash.get(tmppath_path_key, None) + if tmp_path is None: + return rep + + if rep.when == "setup": + # A skipped fixture setup is not a failed test, so preserve the + # behavior from #10502 and mark its temporary directory for cleanup. + if rep.skipped: + item.stash[tmppath_setup_skipped_key] = True + elif rep.when == "teardown": + result_dict = item.stash[tmppath_result_key] + setup_skipped = item.stash.get(tmppath_setup_skipped_key, False) + if ( + setup_skipped + or (result_dict.get("setup", True) and result_dict.get("call", True)) + ) and rep.passed: + # We do a "best effort" to remove files, but it might not be + # possible due to some leaked resource, permissions, etc. + rmtree(tmp_path, ignore_errors=True) + del item.stash[tmppath_path_key] + del item.stash[tmppath_result_key] + if setup_skipped: + del item.stash[tmppath_setup_skipped_key] + return rep diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index 0b33a74b926..d89f6331c32 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -254,6 +254,96 @@ def test_fixt(fixt): base_dir = list(child.iterdir()) assert base_dir == [] + def test_policy_failed_defers_cleanup_until_dependent_teardown( + self, pytester: Pytester + ) -> None: + p = pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def resource(tmp_path): + evidence = tmp_path / "evidence.txt" + evidence.write_text("still here", encoding="utf-8") + yield tmp_path + assert evidence.read_text(encoding="utf-8") == "still here" + + @pytest.fixture + def unavailable(resource): + pytest.skip("optional service unavailable") + + def test_optional_service(unavailable): + pass + """ + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + tmp_path_retention_policy = "failed" + """ + ) + + reprec = pytester.inline_run(p) + reprec.assertoutcome(skipped=1) + + root = pytester._test_tmproot + for child in root.iterdir(): + assert list(child.iterdir()) == [] + + def test_policy_failed_keeps_dir_when_setup_or_teardown_fails( + self, pytester: Pytester + ) -> None: + p = pytester.makepyfile( + """ + import pytest + + def write_evidence(tmp_path, phase): + evidence = tmp_path / "debug-evidence.txt" + evidence.write_text(f"keep me: {phase}", encoding="utf-8") + + @pytest.fixture + def broken_during_setup(tmp_path): + write_evidence(tmp_path, "setup") + raise RuntimeError("setup failed") + + def test_setup_error(broken_during_setup): + pass + + def test_call_failure(tmp_path): + write_evidence(tmp_path, "call") + assert False + + @pytest.fixture + def broken_during_teardown(tmp_path): + write_evidence(tmp_path, "teardown") + yield + raise RuntimeError("teardown failed") + + def test_teardown_error(broken_during_teardown): + pass + """ + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + tmp_path_retention_policy = "failed" + """ + ) + + reprec = pytester.inline_run(p) + reprec.assertoutcome(passed=1, failed=3) + + root = pytester._test_tmproot + evidence = { + path.read_text(encoding="utf-8") + for path in root.rglob("debug-evidence.txt") + } + assert evidence == { + "keep me: setup", + "keep me: call", + "keep me: teardown", + } + # issue #10502 def test_policy_all_keeps_dir_when_skipped_from_fixture( self, pytester: Pytester