Skip to content

Commit

Permalink
Fix rmtree to remove directories with read-only files
Browse files Browse the repository at this point in the history
  • Loading branch information
nicoddemus committed Jul 10, 2019
1 parent 2c402f4 commit 75eefd6
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 3 deletions.
2 changes: 2 additions & 0 deletions changelog/5524.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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.
40 changes: 37 additions & 3 deletions src/_pytest/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
54 changes: 54 additions & 0 deletions testing/test_tmpdir.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import os
import stat
import sys

import attr
Expand Down Expand Up @@ -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"))
Expand All @@ -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

0 comments on commit 75eefd6

Please sign in to comment.