From d38077106556184026137fe83d8351970f08ac89 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 14 Apr 2023 13:24:12 -0300 Subject: [PATCH] Fix tmp_path regression introduced in 7.3.0 (#10911) The problem is that we would loop over all directories of the basetemp directory searching for dead symlinks, for each test, which would compound over the test session run. Doing the cleanup just once, at the end of the session, fixes the problem. Fix #10896 --- changelog/10896.bugfix.rst | 1 + src/_pytest/pathlib.py | 4 ++-- src/_pytest/tmpdir.py | 21 ++++++++++----------- 3 files changed, 13 insertions(+), 13 deletions(-) create mode 100644 changelog/10896.bugfix.rst diff --git a/changelog/10896.bugfix.rst b/changelog/10896.bugfix.rst new file mode 100644 index 000000000..87af0e301 --- /dev/null +++ b/changelog/10896.bugfix.rst @@ -0,0 +1 @@ +Fixed performance regression related to :fixture:`tmp_path` and the new :confval:`tmp_path_retention_policy` option. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 30ed76cf8..2c9d5870b 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -353,7 +353,7 @@ def cleanup_candidates(root: Path, prefix: str, keep: int) -> Iterator[Path]: yield path -def cleanup_dead_symlink(root: Path): +def cleanup_dead_symlinks(root: Path): for left_dir in root.iterdir(): if left_dir.is_symlink(): if not left_dir.resolve().exists(): @@ -371,7 +371,7 @@ def cleanup_numbered_dir( for path in root.glob("garbage-*"): try_cleanup(path, consider_lock_dead_if_created_before) - cleanup_dead_symlink(root) + cleanup_dead_symlinks(root) def make_numbered_dir_with_cleanup( diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 5f347665f..d7f5ab9b4 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -28,7 +28,7 @@ from .pathlib import LOCK_TIMEOUT from .pathlib import make_numbered_dir from .pathlib import make_numbered_dir_with_cleanup from .pathlib import rm_rf -from .pathlib import cleanup_dead_symlink +from .pathlib import cleanup_dead_symlinks from _pytest.compat import final, get_user_id from _pytest.config import Config from _pytest.config import ExitCode @@ -289,31 +289,30 @@ def tmp_path( del request.node.stash[tmppath_result_key] - # remove dead symlink - basetemp = tmp_path_factory._basetemp - if basetemp is None: - return - cleanup_dead_symlink(basetemp) - def pytest_sessionfinish(session, exitstatus: Union[int, ExitCode]): """After each session, remove base directory if all the tests passed, the policy is "failed", and the basetemp is not specified by a user. """ tmp_path_factory: TempPathFactory = session.config._tmp_path_factory - if tmp_path_factory._basetemp is None: + basetemp = tmp_path_factory._basetemp + if basetemp is None: return + policy = tmp_path_factory._retention_policy if ( exitstatus == 0 and policy == "failed" and tmp_path_factory._given_basetemp is None ): - passed_dir = tmp_path_factory._basetemp - if passed_dir.exists(): + if basetemp.is_dir(): # 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(passed_dir, ignore_errors=True) + rmtree(basetemp, ignore_errors=True) + + # Remove dead symlinks. + if basetemp.is_dir(): + cleanup_dead_symlinks(basetemp) @hookimpl(tryfirst=True, hookwrapper=True)