From 5d32623b5471a7be1255ba4f3d4f4e8666ad2d98 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 | 11 ++++++--- testing/test_tmpdir.py | 48 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 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..74fa65eb55c 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -37,10 +37,15 @@ def ensure_reset_dir(path): def rmtree(path, force=False): + def force_writable_and_retry(function, path, excinfo): + import stat + + mode = os.stat(path).st_mode + os.chmod(path, mode | stat.S_IWRITE) + function(path) + 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) + shutil.rmtree(str(path), onerror=force_writable_and_retry) else: shutil.rmtree(str(path)) diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index c4c7ebe256e..793095cdbd3 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,32 @@ 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() + os.chmod(str(fn), stat.S_IREAD) + 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() + os.chmod(str(adir), stat.S_IREAD) + 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 +377,23 @@ 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') + os.chmod(str(fn), 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