From f20eeebde94f12424145f8548c960081d2e3c3ab Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 30 Oct 2018 13:31:55 -0300 Subject: [PATCH] Fix access denied error when deleting a stale temporary directory Fix #4262 --- changelog/4262.bugfix.rst | 1 + src/_pytest/pathlib.py | 24 +++++++++++++++------- testing/{test_paths.py => test_pathlib.py} | 18 ++++++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) create mode 100644 changelog/4262.bugfix.rst rename testing/{test_paths.py => test_pathlib.py} (78%) diff --git a/changelog/4262.bugfix.rst b/changelog/4262.bugfix.rst new file mode 100644 index 000000000..1487138b7 --- /dev/null +++ b/changelog/4262.bugfix.rst @@ -0,0 +1 @@ +Fix access denied error when deleting stale directories created by ``tmpdir`` / ``tmp_path``. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 7cf3f40b6..0ace312e9 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -186,19 +186,29 @@ def register_cleanup_lock_removal(lock_path, register=atexit.register): def maybe_delete_a_numbered_dir(path): - """removes a numbered directory if its lock can be obtained""" + """removes a numbered directory if its lock can be obtained and it does not seem to be in use""" + lock_path = None try: - create_cleanup_lock(path) + lock_path = create_cleanup_lock(path) + parent = path.parent + + garbage = parent.joinpath("garbage-{}".format(uuid.uuid4())) + path.rename(garbage) + rmtree(garbage, force=True) except (OSError, EnvironmentError): # known races: # * other process did a cleanup at the same time # * deletable folder was found + # * process cwd (Windows) return - parent = path.parent - - garbage = parent.joinpath("garbage-{}".format(uuid.uuid4())) - path.rename(garbage) - rmtree(garbage, force=True) + finally: + # if we created the lock, ensure we remove it even if we failed + # to properly remove the numbered dir + if lock_path is not None: + try: + lock_path.unlink() + except (OSError, IOError): + pass def ensure_deletable(path, consider_lock_dead_if_created_before): diff --git a/testing/test_paths.py b/testing/test_pathlib.py similarity index 78% rename from testing/test_paths.py rename to testing/test_pathlib.py index 65ee9b634..8ac404070 100644 --- a/testing/test_paths.py +++ b/testing/test_pathlib.py @@ -4,6 +4,9 @@ import py import pytest from _pytest.pathlib import fnmatch_ex +from _pytest.pathlib import get_lock_path +from _pytest.pathlib import maybe_delete_a_numbered_dir +from _pytest.pathlib import Path class TestPort: @@ -66,3 +69,18 @@ class TestPort: ) def test_not_matching(self, match, pattern, path): assert not match(pattern, path) + + +def test_access_denied_during_cleanup(tmp_path, monkeypatch): + """Ensure that deleting a numbered dir does not fail because of OSErrors (#4262).""" + path = tmp_path / "temp-1" + path.mkdir() + + def renamed_failed(*args): + raise OSError("access denied") + + monkeypatch.setattr(Path, "rename", renamed_failed) + + lock_path = get_lock_path(path) + maybe_delete_a_numbered_dir(path) + assert not lock_path.is_file()