Skip to content

Commit

Permalink
Mock out lchmod functions in _patch_for_wrapping_test?
Browse files Browse the repository at this point in the history
TestRmtree._patch_for_wrapping_test already mocked out the regular
chmod functions in the os module and the Path class, to test what
happens when changing permissions cannot fix an error. But there
are also, on some systems and Python versions, lchmod versions of
these functions. This patches those as well.

I am not sure this should really be done. The problem is that
calling such functions is fairly likely to raise an exception if
it is not properly conditioned on a check for their actual
usability, and mocking them out could obscure such a bug.
  • Loading branch information
EliahKagan committed Nov 29, 2023
1 parent 5c6a4f4 commit e309b35
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,15 @@ def _patch_for_wrapping_test(self, mocker, hide_windows_known_errors):
# git.index.util "replaces" git.util and is what "import git.util" gives us.
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", hide_windows_known_errors)

# Disable common chmod functions so the callback can't fix a PermissionError.
# Disable some chmod functions so the callback can't fix a PermissionError.
mocker.patch.object(os, "chmod")
if hasattr(os, "lchmod"):
# Exists on some operating systems. Mocking out os.chmod doesn't affect it.
mocker.patch.object(os, "lchmod")
mocker.patch.object(pathlib.Path, "chmod")
if hasattr(pathlib.Path, "lchmod"):
# Exists on some Python versions. Don't rely on it calling a public chmod.
mocker.patch.object(pathlib.Path, "lchmod")

@pytest.mark.skipif(
os.name != "nt",
Expand Down

0 comments on commit e309b35

Please sign in to comment.