Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-119993 ignore NotADirectoryError in Path.unlink() if missing_ok is True #120049

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
14 changes: 10 additions & 4 deletions Doc/library/pathlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1630,15 +1630,21 @@ Copying, renaming and deleting
Remove this file or symbolic link. If the path points to a directory,
use :func:`Path.rmdir` instead.

If *missing_ok* is false (the default), :exc:`FileNotFoundError` is
raised if the path does not exist.
If *missing_ok* is false (the default), this method propagates any
:exc:`OSError` from the operating system, including :exc:`FileNotFoundError`.
MusicalNinjaDad marked this conversation as resolved.
Show resolved Hide resolved

If *missing_ok* is true, :exc:`FileNotFoundError` exceptions will be
ignored (same behavior as the POSIX ``rm -f`` command).
If *missing_ok* is true, this shows similar behavior to the POSIX ``rm -f``
command and any :exc:`FileNotFoundError` or :exc:`NotADirectoryError`
exceptions will be ignored. This means that the file does not exist after
execution, but cannot guarantee that the file did exist before. Any other
:exc:`OSError` which is encountered will continue to be propogated.
MusicalNinjaDad marked this conversation as resolved.
Show resolved Hide resolved

.. versionchanged:: 3.8
The *missing_ok* parameter was added.

.. versionchanged:: 3.??
The *missing_ok* parameter will also ignore :exc:`NotADirectoryError`

MusicalNinjaDad marked this conversation as resolved.
Show resolved Hide resolved

.. method:: Path.rmdir()

Expand Down
2 changes: 1 addition & 1 deletion Lib/pathlib/_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ def unlink(self, missing_ok=False):
"""
try:
os.unlink(self)
except FileNotFoundError:
except (FileNotFoundError, NotADirectoryError):
if not missing_ok:
raise

Expand Down
29 changes: 29 additions & 0 deletions Lib/test/test_pathlib/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,35 @@ def test_group_no_follow_symlinks(self):
self.assertEqual(expected_gid, gid_2)
self.assertEqual(expected_name, link.group(follow_symlinks=False))

def test_unlink(self):
p = self.cls(self.base) / 'fileA'
p.unlink()
self.assertFileNotFound(p.stat)
self.assertFileNotFound(p.unlink)

def test_unlink_missing_ok(self):
p = self.cls(self.base) / 'fileAAA'
self.assertFileNotFound(p.unlink)
p.unlink(missing_ok=True)

def test_unlink_missing_ok_intermediate_file(self):
MusicalNinjaDad marked this conversation as resolved.
Show resolved Hide resolved
p = self.cls(self.base) / 'fileAAA'
p.touch()
p = p / 'fileBBB'
if sys.platform.startswith("win"):
self.assertFileNotFound(p.unlink)
Comment on lines +796 to +797
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a @needs_posix decorator to the test and don't bother testing Windows IMO :]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this hurt to have it in? Isn't it generally better to have the extra test coverage, or are we really struggling to keep execution times down?

else:
self.assertNotADirectory(p.unlink)
p.unlink(missing_ok=True)

def test_rmdir(self):
p = self.cls(self.base) / 'dirA'
for q in p.iterdir():
q.unlink()
p.rmdir()
self.assertFileNotFound(p.stat)
self.assertFileNotFound(p.unlink)

@os_helper.skip_unless_hardlink
def test_hardlink_to(self):
P = self.cls(self.base)
Expand Down
5 changes: 5 additions & 0 deletions Lib/test/test_pathlib/test_pathlib_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,11 @@ def assertFileNotFound(self, func, *args, **kwargs):
func(*args, **kwargs)
self.assertEqual(cm.exception.errno, errno.ENOENT)

def assertNotADirectory(self, func, *args, **kwargs):
with self.assertRaises(NotADirectoryError) as cm:
func(*args, **kwargs)
self.assertEqual(cm.exception.errno, errno.ENOTDIR)

def assertEqualNormCase(self, path_a, path_b):
normcase = self.parser.normcase
self.assertEqual(normcase(path_a), normcase(path_b))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:meth:`pathlib.Path.unlink` will also ignore any :exc:`NotADirectoryError`
if *missing_ok* is true.
Loading