From ceea364afbcacfa063d9c0c193a4bdb7a3207893 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 | 11 +++++------ testing/test_pathlib.py | 20 ++++++++++++++++++++ 3 files changed, 26 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 000000000..141a0be04 --- /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 0bc5bff2b..047600c7d 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -6,7 +6,7 @@ import itertools import os import shutil import sys -import uuid +import tempfile import warnings from enum import Enum from functools import partial @@ -254,11 +254,10 @@ 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 e37b33847..4be73f2d6 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -351,6 +351,26 @@ def test_long_path_during_cleanup(tmp_path): assert not os.path.isdir(extended_path) +def test_permission_denied_during_cleanup(tmp_path): + """ + 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 p in p.parents: + if p == path: + break + p.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(): 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"