From 75eefd67c29ca9b210a50f8af08b49777b8802b9 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 10 Jul 2019 09:52:23 -0300 Subject: [PATCH] Fix rmtree to remove directories with read-only files Fix #5524 --- changelog/5524.bugfix.rst | 2 ++ src/_pytest/pathlib.py | 40 ++++++++++++++++++++++++++--- testing/test_tmpdir.py | 54 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 changelog/5524.bugfix.rst diff --git a/changelog/5524.bugfix.rst b/changelog/5524.bugfix.rst new file mode 100644 index 00000000000..96ebbd43e09 --- /dev/null +++ b/changelog/5524.bugfix.rst @@ -0,0 +1,2 @@ +Fix issue where ``tmp_path`` and ``tmpdir`` would not remove directories containing files marked as read-only, +which could lead to pytest crashing when executed a second time with the ``--basetemp`` option. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index ecc38eb0f4d..2a1d0089582 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -38,13 +38,47 @@ def ensure_reset_dir(path): def rmtree(path, force=False): if force: - # NOTE: ignore_errors might leave dead folders around. - # Python needs a rm -rf as a followup. - shutil.rmtree(str(path), ignore_errors=True) + rm_rf(path) else: shutil.rmtree(str(path)) +def rm_rf(path): + """Remove the path contents recursively, even if some elements + are read-only. + """ + + def chmod_w(p): + import stat + + mode = os.stat(str(p)).st_mode + os.chmod(str(p), mode | stat.S_IWRITE) + + def force_writable_and_retry(function, p, excinfo): + p = Path(p) + + # for files, we need to recursively go upwards + # in the directories to ensure they all are also + # writable + if p.is_file(): + for parent in p.parents: + chmod_w(parent) + # stop when we reach the original path passed to rm_rf + if parent == path: + break + + chmod_w(p) + try: + # retry the function that failed + function(p) + except Exception: + # need to silently ignore this error to preserve the + # previous behavior of rmtree(..., force=True) + pass + + shutil.rmtree(str(path), onerror=force_writable_and_retry) + + def find_prefixed(root, prefix): """finds all elements in root that begin with the prefix, case insensitive""" l_prefix = prefix.lower() diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index c4c7ebe256e..8e4cc81b889 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -1,3 +1,5 @@ +import os +import stat import sys import attr @@ -326,6 +328,37 @@ def test_rmtree(self, tmp_path): rmtree(adir, force=True) assert not adir.exists() + def test_rmtree_with_read_only_file(self, tmp_path): + """Ensure rmtree can remove directories with read-only files in them (#5524)""" + from _pytest.pathlib import rmtree + + fn = tmp_path / "dir/foo.txt" + fn.parent.mkdir() + + fn.touch() + + mode = os.stat(str(fn)).st_mode + os.chmod(str(fn), mode & ~stat.S_IWRITE) + + rmtree(fn.parent, force=True) + + assert not fn.parent.is_dir() + + def test_rmtree_with_read_only_directory(self, tmp_path): + """Ensure rmtree can remove read-only directories (#5524)""" + from _pytest.pathlib import rmtree + + adir = tmp_path / "dir" + adir.mkdir() + + (adir / "foo.txt").touch() + mode = os.stat(str(adir)).st_mode + os.chmod(str(adir), mode & ~stat.S_IWRITE) + + rmtree(adir, force=True) + + assert not adir.is_dir() + def test_cleanup_ignores_symlink(self, tmp_path): the_symlink = tmp_path / (self.PREFIX + "current") attempt_symlink_to(the_symlink, tmp_path / (self.PREFIX + "5")) @@ -349,3 +382,24 @@ def attempt_symlink_to(path, to_path): def test_tmpdir_equals_tmp_path(tmpdir, tmp_path): assert Path(tmpdir) == tmp_path + + +def test_basetemp_with_read_only_files(testdir): + """Integration test for #5524""" + testdir.makepyfile( + """ + import os + import stat + + def test(tmp_path): + fn = tmp_path / 'foo.txt' + fn.write_text('hello') + mode = os.stat(str(fn)).st_mode + os.chmod(str(fn), mode & ~stat.S_IREAD) + """ + ) + result = testdir.runpytest("--basetemp=tmp") + assert result.ret == 0 + # running a second time and ensure we don't crash + result = testdir.runpytest("--basetemp=tmp") + assert result.ret == 0