Skip to content

Commit 8a22352

Browse files
committed
Extract some shared patching code to a helper
Because of how it is parameterized, but only in some of the test cases that use it, making this a pytest fixture would be cumbersome without further refactoring, so this is a helper method instead. (It may be feasible to further refactor the test suite to improve this more.) This also removes a type annotation from a test method. It may eventually be helpful to add annotations, in which case that would come back along with others (if that test still exists in this form), but it's the only test with such an annotation in this test class, and the annotation had been added accidentally.
1 parent e8cfae8 commit 8a22352

File tree

1 file changed

+14
-19
lines changed

1 file changed

+14
-19
lines changed

Diff for: test/test_util.py

+14-19
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ def test_deletes_dir_with_readonly_files(self, tmp_path):
109109
sys.platform == "cygwin",
110110
reason="Cygwin can't set the permissions that make the test meaningful.",
111111
)
112-
def test_avoids_changing_permissions_outside_tree(self, tmp_path: pathlib.Path):
112+
def test_avoids_changing_permissions_outside_tree(self, tmp_path):
113113
# Automatically works on Windows, but on Unix requires either special handling
114114
# or refraining from attempting to fix PermissionError by making chmod calls.
115115

@@ -134,22 +134,24 @@ def test_avoids_changing_permissions_outside_tree(self, tmp_path: pathlib.Path):
134134
new_mode = (dir1 / "file").stat().st_mode
135135
assert old_mode == new_mode, f"Should stay {old_mode:#o}, became {new_mode:#o}."
136136

137-
@pytest.mark.skipif(
138-
os.name != "nt",
139-
reason="PermissionError is only ever wrapped on Windows",
140-
)
141-
def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir):
142-
"""rmtree wraps PermissionError on Windows when HIDE_WINDOWS_KNOWN_ERRORS is true."""
137+
def _patch_for_wrapping_test(self, mocker, hide_windows_known_errors):
143138
# Access the module through sys.modules so it is unambiguous which module's
144139
# attribute we patch: the original git.util, not git.index.util even though
145140
# git.index.util "replaces" git.util and is what "import git.util" gives us.
146-
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", True)
141+
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", hide_windows_known_errors)
147142

148-
# Disable common chmod functions so the callback can never fix the problem.
143+
# Disable common chmod functions so the callback can't fix a PermissionError.
149144
mocker.patch.object(os, "chmod")
150145
mocker.patch.object(pathlib.Path, "chmod")
151146

152-
# Now we can see how an intractable PermissionError is treated.
147+
@pytest.mark.skipif(
148+
os.name != "nt",
149+
reason="PermissionError is only ever wrapped on Windows",
150+
)
151+
def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir):
152+
"""rmtree wraps PermissionError on Windows when HIDE_WINDOWS_KNOWN_ERRORS is true."""
153+
self._patch_for_wrapping_test(mocker, True)
154+
153155
with pytest.raises(SkipTest):
154156
rmtree(permission_error_tmpdir)
155157

@@ -166,10 +168,7 @@ def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir):
166168
)
167169
def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_tmpdir, hide_windows_known_errors):
168170
"""rmtree does not wrap PermissionError on non-Windows systems or when HIDE_WINDOWS_KNOWN_ERRORS is false."""
169-
# See comments in test_wraps_perm_error_if_enabled for details about patching.
170-
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", hide_windows_known_errors)
171-
mocker.patch.object(os, "chmod")
172-
mocker.patch.object(pathlib.Path, "chmod")
171+
self._patch_for_wrapping_test(mocker, hide_windows_known_errors)
173172

174173
with pytest.raises(PermissionError):
175174
try:
@@ -180,11 +179,7 @@ def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_
180179
@pytest.mark.parametrize("hide_windows_known_errors", [False, True])
181180
def test_does_not_wrap_other_errors(self, tmp_path, mocker, hide_windows_known_errors):
182181
file_not_found_tmpdir = tmp_path / "testdir" # It is deliberately never created.
183-
184-
# See comments in test_wraps_perm_error_if_enabled for details about patching.
185-
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", hide_windows_known_errors)
186-
mocker.patch.object(os, "chmod")
187-
mocker.patch.object(pathlib.Path, "chmod")
182+
self._patch_for_wrapping_test(mocker, hide_windows_known_errors)
188183

189184
with pytest.raises(FileNotFoundError):
190185
try:

0 commit comments

Comments
 (0)