Content-Length: 5640 | pFad | http://github.com/gitpython-developers/GitPython/pull/1749.patch

thub.com From e309b35dc216db4596374e1c1a5730e2690deb77 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 13 Nov 2023 22:44:08 -0500 Subject: [PATCH 1/3] Mock out lchmod functions in _patch_for_wrapping_test? 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. --- test/test_util.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/test_util.py b/test/test_util.py index e5a5a0557..e003fcb05 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -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", From a09e5383cc9bbba290c18c22d925065453b47747 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 28 Nov 2023 20:47:38 -0500 Subject: [PATCH 2/3] Don't mock the lchmod functions, and explain why This undoes the mocking of lchmod functions from e309b35, and instead notes why they may be better left alone in the tests. This also rewords the existing comment to better explain the reason for the mocking that is being done. --- test/test_util.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/test_util.py b/test/test_util.py index e003fcb05..68f75aa45 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -140,15 +140,11 @@ 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 some chmod functions so the callback can't fix a PermissionError. + # Mock out common chmod functions to simulate PermissionError the callback can't + # fix. (We leave the corresponding lchmod functions alone. If they're used, it's + # more important we detect any failures from inadequate compatibility checks.) 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", From 2fabe71b7ba6feb5e63795c51bf60a1361ed936d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 28 Nov 2023 23:00:29 -0500 Subject: [PATCH 3/3] Further document cygpath test parameter collections --- test/test_util.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_util.py b/test/test_util.py index 68f75aa45..f1ce17bca 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -258,14 +258,15 @@ def _xfail_param(*values, **xfail_kwargs): (R"D:/Apps\fOO", "/cygdrive/d/Apps/fOO"), (R"D:\Apps/123", "/cygdrive/d/Apps/123"), ) +"""Path test cases for cygpath and decygpath, other than extended UNC paths.""" _unc_cygpath_pairs = ( (R"\\?\a:\com", "/cygdrive/a/com"), (R"\\?\a:/com", "/cygdrive/a/com"), (R"\\?\UNC\server\D$\Apps", "//server/D$/Apps"), ) +"""Extended UNC path test cases for cygpath.""" -# Mapping of expected failures for the test_cygpath_ok test. _cygpath_ok_xfails = { # From _norm_cygpath_pairs: (R"C:\Users", "/cygdrive/c/Users"): "/proc/cygdrive/c/Users", @@ -279,9 +280,9 @@ def _xfail_param(*values, **xfail_kwargs): (R"\\?\a:\com", "/cygdrive/a/com"): "/proc/cygdrive/a/com", (R"\\?\a:/com", "/cygdrive/a/com"): "/proc/cygdrive/a/com", } +"""Mapping of expected failures for the test_cygpath_ok test.""" -# Parameter sets for the test_cygpath_ok test. _cygpath_ok_params = [ ( _xfail_param(*case, reason=f"Returns: {_cygpath_ok_xfails[case]!r}", raises=AssertionError) @@ -290,6 +291,7 @@ def _xfail_param(*values, **xfail_kwargs): ) for case in _norm_cygpath_pairs + _unc_cygpath_pairs ] +"""Parameter sets for the test_cygpath_ok test.""" @pytest.mark.skipif(sys.platform != "cygwin", reason="Paths specifically for Cygwin.")

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!