From f78b56aa39e41d8b461f1d05d7795168bff4e301 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 26 Oct 2020 12:49:35 +0000 Subject: [PATCH] use tempfile.TemporaryDirectory to cleanup garbage- dir Python has a very nice rm_rf hidden in tempfile.TemporaryDirectory._rmtree we can use it to cleanup our "garbage-" dir https://github.com/python/cpython/blob/3d43f1dce3e9aaded38f9a2c73e3c66acf85505c/Lib/tempfile.py#L784-L812 --- changelog/8940.bugfix.rst | 1 + src/_pytest/pathlib.py | 9 +++------ testing/test_pathlib.py | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 changelog/8940.bugfix.rst diff --git a/changelog/8940.bugfix.rst b/changelog/8940.bugfix.rst new file mode 100644 index 00000000000..141a0be0415 --- /dev/null +++ b/changelog/8940.bugfix.rst @@ -0,0 +1 @@ +Fixed an issue where dirs without S_IXUSR prevented pytest from cleaning up the temp dir diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index cd15434605d..4af9f49c7f8 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -26,11 +26,11 @@ from posixpath import sep as posix_sep import shutil import sys +import tempfile import types from types import ModuleType from typing import Any from typing import TypeVar -import uuid import warnings from _pytest.compat import assert_never @@ -286,11 +286,8 @@ def maybe_delete_a_numbered_dir(path: Path) -> None: lock_path = None try: lock_path = create_cleanup_lock(path) - parent = path.parent - - garbage = parent.joinpath(f"garbage-{uuid.uuid4()}") - path.rename(garbage) - rm_rf(garbage) + with tempfile.TemporaryDirectory(prefix="garbage-", dir=path.parent) as garbage: + path.replace(garbage) except OSError: # known races: # * other process did a cleanup at the same time diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index 0880c355557..8993b463af9 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -486,6 +486,26 @@ def test_long_path_during_cleanup(tmp_path: Path) -> None: assert not os.path.isdir(extended_path) +def test_permission_denied_during_cleanup(tmp_path: Path) -> None: + """ + Ensure that deleting a numbered dir does not fail because of missing file + permission bits (#7940). + """ + path = tmp_path / "temp-1" + p = path / "ham" / "spam" / "eggs" + p.parent.mkdir(parents=True) + p.touch(mode=0) + for parent in p.parents: + if parent == path: + break + parent.chmod(mode=0) + + lock_path = get_lock_path(path) + maybe_delete_a_numbered_dir(path) + assert not path.exists() + assert not lock_path.is_file() + + def test_get_extended_length_path_str() -> None: assert get_extended_length_path_str(r"c:\foo") == r"\\?\c:\foo" assert get_extended_length_path_str(r"\\share\foo") == r"\\?\UNC\share\foo"