Merge pull request #4267 from nicoddemus/tmpdir-4262

Fix access denied error when deleting a stale temporary directory

Author: blueyed

changelog/4262.bugfix.rst

@@ -0,0 +1 @@
+Fix access denied error when deleting stale directories created by ``tmpdir`` / ``tmp_path``.

src/_pytest/pathlib.py

@@ -186,19 +186,29 @@ def cleanup_on_exit(lock_path=lock_path, original_pid=pid):
 
 
 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):

testing/test_paths.py renamed to testing/test_pathlib.py

@@ -4,6 +4,9 @@
 
 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 @@ def test_matching(self, match, pattern, path):
     )
     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()