Skip to content

Commit

Permalink
Test that PermissionError is only wrapped on Windows
Browse files Browse the repository at this point in the history
This changes tests in test_util to verify the opposite behavior
from what was enforced before, in the unusual case (that hopefully
never happens outside of monkey-patching in test_util.py itself)
where the system is not Windows but HIDE_WINDOWS_KNOWN_ERRORS is
set to True.

The same-named environment variable will not, and never has, set
HIDE_WINDOWS_KNOWN_ERRORS to True on non-Windows systems, but it is
possible to set it to True directly. Since it is named as a
constant and no documentation has ever suggested changing its value
directly, nor otherwise attempting to use it outside Windows, it
shouldn't matter what happens in this unusual case. But asserting
that wrapping never occurs in this combination of circumstances is
what makes the most sense in light of the recent change to pass no
callback to shutil.rmtree on non-Windows systems.
  • Loading branch information
EliahKagan committed Nov 14, 2023
1 parent 6de8e67 commit e8cfae8
Showing 1 changed file with 13 additions and 6 deletions.
19 changes: 13 additions & 6 deletions test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ def test_avoids_changing_permissions_outside_tree(self, tmp_path: pathlib.Path):
assert old_mode == new_mode, f"Should stay {old_mode:#o}, became {new_mode:#o}."

@pytest.mark.skipif(
sys.platform == "cygwin",
reason="Cygwin can't set the permissions that make the test meaningful.",
os.name != "nt",
reason="PermissionError is only ever wrapped on Windows",
)
def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir):
"""rmtree wraps PermissionError when HIDE_WINDOWS_KNOWN_ERRORS is true."""
"""rmtree wraps PermissionError on Windows when HIDE_WINDOWS_KNOWN_ERRORS is true."""
# Access the module through sys.modules so it is unambiguous which module's
# attribute we patch: the original git.util, not git.index.util even though
# git.index.util "replaces" git.util and is what "import git.util" gives us.
Expand All @@ -157,10 +157,17 @@ def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir):
sys.platform == "cygwin",
reason="Cygwin can't set the permissions that make the test meaningful.",
)
def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_tmpdir):
"""rmtree does not wrap PermissionError when HIDE_WINDOWS_KNOWN_ERRORS is false."""
@pytest.mark.parametrize(
"hide_windows_known_errors",
[
pytest.param(False),
pytest.param(True, marks=pytest.mark.skipif(os.name == "nt", reason="We would wrap on Windows")),
],
)
def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_tmpdir, hide_windows_known_errors):
"""rmtree does not wrap PermissionError on non-Windows systems or when HIDE_WINDOWS_KNOWN_ERRORS is false."""
# See comments in test_wraps_perm_error_if_enabled for details about patching.
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", False)
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", hide_windows_known_errors)
mocker.patch.object(os, "chmod")
mocker.patch.object(pathlib.Path, "chmod")

Expand Down

0 comments on commit e8cfae8

Please sign in to comment.