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

thub.com From 85ef145b06e6eb164bc469f72b96a917c567c85a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 6 Feb 2024 01:01:50 -0500 Subject: [PATCH 1/5] Extend test_cmd_override to test exception's `command` attribute --- test/test_git.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/test_git.py b/test/test_git.py index ea3d067ee..1148ccfd8 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -321,17 +321,17 @@ def test_version(self): self.assertIsInstance(n, int) # END verify number types - def test_cmd_override(self): - with mock.patch.object( - type(self.git), - "GIT_PYTHON_GIT_EXECUTABLE", - osp.join("some", "path", "which", "doesn't", "exist", "gitbinary"), - ): - self.assertRaises(GitCommandNotFound, self.git.version) - def test_git_exc_name_is_git(self): self.assertEqual(self.git.git_exec_name, "git") + def test_cmd_override(self): + """Directly set bad GIT_PYTHON_GIT_EXECUTABLE causes git operations to raise.""" + bad_path = osp.join("some", "path", "which", "doesn't", "exist", "gitbinary") + with mock.patch.object(type(self.git), "GIT_PYTHON_GIT_EXECUTABLE", bad_path): + with self.assertRaises(GitCommandNotFound) as ctx: + self.git.version() + self.assertEqual(ctx.exception.command, [bad_path, "version"]) + @ddt.data(("0",), ("q",), ("quiet",), ("s",), ("silence",), ("silent",), ("n",), ("none",)) def test_initial_refresh_from_bad_git_path_env_quiet(self, case): """In "q" mode, bad initial path sets "git" and is quiet.""" From db361743a52455ce064d2fbcb766ec9979960d5a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 6 Feb 2024 01:05:07 -0500 Subject: [PATCH 2/5] Write Git instead of type(self.git) Some tests in test_git.TestGit used type(self.git) while others used Git. This changes them all to use Git. The distinction could be relevant if self.git were set as a mock, but this is not being done, and if it were, it is not obvious which would be preferred. The intention of the existing tests is to test attributes of the Git class, so this picks that simpler expression. --- test/test_git.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/test_git.py b/test/test_git.py index 1148ccfd8..b02a92b0f 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -327,7 +327,7 @@ def test_git_exc_name_is_git(self): def test_cmd_override(self): """Directly set bad GIT_PYTHON_GIT_EXECUTABLE causes git operations to raise.""" bad_path = osp.join("some", "path", "which", "doesn't", "exist", "gitbinary") - with mock.patch.object(type(self.git), "GIT_PYTHON_GIT_EXECUTABLE", bad_path): + with mock.patch.object(Git, "GIT_PYTHON_GIT_EXECUTABLE", bad_path): with self.assertRaises(GitCommandNotFound) as ctx: self.git.version() self.assertEqual(ctx.exception.command, [bad_path, "version"]) @@ -341,7 +341,7 @@ def test_initial_refresh_from_bad_git_path_env_quiet(self, case): "GIT_PYTHON_REFRESH": mode, } with _rollback_refresh(): - type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. + Git.GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. with mock.patch.dict(os.environ, set_vars): refresh() @@ -356,7 +356,7 @@ def test_initial_refresh_from_bad_git_path_env_warn(self, case): "GIT_PYTHON_REFRESH": mode, } with _rollback_refresh(): - type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. + Git.GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. with mock.patch.dict(os.environ, env_vars): with self.assertLogs(cmd.__name__, logging.CRITICAL) as ctx: @@ -375,7 +375,7 @@ def test_initial_refresh_from_bad_git_path_env_error(self, case): "GIT_PYTHON_REFRESH": mode, } with _rollback_refresh(): - type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. + Git.GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. with mock.patch.dict(os.environ, env_vars): with self.assertRaisesRegex(ImportError, r"\ABad git executable.\n"): @@ -386,7 +386,7 @@ def test_initial_refresh_from_good_absolute_git_path_env(self): absolute_path = shutil.which("git") with _rollback_refresh(): - type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. + Git.GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": absolute_path}): refresh() @@ -397,8 +397,8 @@ def test_initial_refresh_from_good_relative_git_path_env(self): with _rollback_refresh(): # Set the fallback to a string that wouldn't work and isn't "git", so we are # more likely to detect if "git" is not set from the environment variable. - with mock.patch.object(type(self.git), "git_exec_name", ""): - type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. + with mock.patch.object(Git, "git_exec_name", ""): + Git.GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup. # Now observe if setting the environment variable to "git" takes effect. with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": "git"}): @@ -442,7 +442,7 @@ def test_refresh_from_good_relative_git_path_env(self): """Good relative path from environment is kept relative and set.""" with _rollback_refresh(): # Set as the executable name a string that wouldn't work and isn't "git". - type(self.git).GIT_PYTHON_GIT_EXECUTABLE = "" + Git.GIT_PYTHON_GIT_EXECUTABLE = "" # Now observe if setting the environment variable to "git" takes effect. with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": "git"}): From 3250313d95e37040327c4d8620d03ec4ffc494df Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 6 Feb 2024 01:57:53 -0500 Subject: [PATCH 3/5] Test that extra prefix "WARNING:" is omitted This changes test_initial_refresh_from_bad_git_path_env_warn to assert a message with no added "WARNING:" prefix. As discussed in #1815, the extra prefix is confusing with logging configured, showing "CRITICAL:git.cmd:WARNING: Bad git executable." instead of "CRITICAL:git.cmd: Bad git executable." --- test/test_git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_git.py b/test/test_git.py index b02a92b0f..f12428f9e 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -363,7 +363,7 @@ def test_initial_refresh_from_bad_git_path_env_warn(self, case): refresh() self.assertEqual(len(ctx.records), 1) message = ctx.records[0].getMessage() - self.assertRegex(message, r"\AWARNING: Bad git executable.\n") + self.assertRegex(message, r"\ABad git executable.\n") self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git") @ddt.data(("2",), ("r",), ("raise",), ("e",), ("error",)) From 5faf62107a44c301e7d42a299926fc707f7299f2 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 6 Feb 2024 02:04:27 -0500 Subject: [PATCH 4/5] Omit extra "WARNING:" prefix As tested for in 3250313. This makes the test pass. --- git/cmd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/cmd.py b/git/cmd.py index 22b5ea99e..f58e6df5b 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -450,7 +450,7 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool: ) if mode in warn: - _logger.critical("WARNING: %s", err) + _logger.critical(err) else: raise ImportError(err) else: From 4b86993e029acf0bb7b36a4b5e40a588bdab6658 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 6 Feb 2024 02:07:14 -0500 Subject: [PATCH 5/5] Use same code style for all logging without placeholders When logging with only a msg argument, it is a full literal message rather than a format string (as it is when there are placeholders). Thus both `...("%s", text)` and `...(text)`, where `...` is a logging method or function, are equally good code styles, provided `text` really is known to behave the same as `str(text)`. The latter style, `...(text)`, was used in all logging calls, both in the git module and in the test suite, except one. This changes the one outlier from `...("%s", text)` to `...(text)` for stylistic consistency and to avoid giving the false impression that there is something special about that call. --- test/test_index.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_index.py b/test/test_index.py index f456f8be0..ffe71450d 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -174,7 +174,7 @@ def _decode(stdout): except UnicodeDecodeError: pass except LookupError as error: - _logger.warning("%s", str(error)) # Message already says "Unknown encoding:". + _logger.warning(str(error)) # Message already says "Unknown encoding:". # Assume UTF-8. If invalid, substitute Unicode replacement characters. return stdout.decode("utf-8", errors="replace")

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