Skip to content

Commit

Permalink
Change warning refresh-mode tests to expect logging
Browse files Browse the repository at this point in the history
This is instead of the current behavior writing the message to
stdout.

This commit does not change the behavior of the code under test,
but it changes tests to assert the following:

- "Bad git executable" messages are logged, at level CRITICAL.
- "log" (and "l") is recognized as another synonym of "warn".
- "silent" is recognized as a synonym of "quiet" (as "silence" is).

Although it is ambiguous whether this should be logged at level
ERROR or CRITICAL, because git.refresh is still expected to be
usable and can be called manually, not having a working git is a
condition in which GitPython, and any program that really relies on
its functionality, should be expected not work. That is the general
rationale for using CRIICAL here. There are also two specific
reasons:

- Existing messages GitPython logs as ERROR typically represent
  errors in individual operations on repositories, which could fail
  without indicating that GitPython's overall functionality is in
  an unusable state. Using the higher CRITICAL level for this
  situation makes sense for contrast.

- Prior to gitpython-developers#1813, logging messsges emitted from GitPython modules,
  no matter the level, were suppressed when logging was not
  configured, but because this message was printed instead of
  being logged, it was still shown. Now that it is to be logged,
  there may be a benefit to have an easy way for someone to bring
  back a close approximation of the old behavior. Having this
  message be at a higher logging level makes that easier to do.
  (This is a less important reason, as that should rarely be done.)

test_initial_refresh_from_bad_git_path_env_warn is the main changed
test. All tests should pass again once code is changed for gitpython-developers#1808.
  • Loading branch information
EliahKagan committed Feb 2, 2024
1 parent 147e80b commit ac50d8e
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions test/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import contextlib
import gc
import inspect
import io
import logging
import os
import os.path as osp
Expand Down Expand Up @@ -333,7 +332,7 @@ def test_cmd_override(self):
def test_git_exc_name_is_git(self):
self.assertEqual(self.git.git_exec_name, "git")

@ddt.data(("0",), ("q",), ("quiet",), ("s",), ("silence",), ("n",), ("none",))
@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."""
(mode,) = case
Expand All @@ -348,9 +347,9 @@ def test_initial_refresh_from_bad_git_path_env_quiet(self, case):
refresh()
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git")

@ddt.data(("1",), ("w",), ("warn",), ("warning",))
@ddt.data(("1",), ("w",), ("warn",), ("warning",), ("l",), ("log",))
def test_initial_refresh_from_bad_git_path_env_warn(self, case):
"""In "w" mode, bad initial path sets "git" and warns."""
"""In "w" mode, bad initial path sets "git" and warns, by logging."""
(mode,) = case
env_vars = {
"GIT_PYTHON_GIT_EXECUTABLE": str(Path("yada").absolute()), # Any bad path.
Expand All @@ -360,9 +359,11 @@ def test_initial_refresh_from_bad_git_path_env_warn(self, case):
type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup.

with mock.patch.dict(os.environ, env_vars):
with contextlib.redirect_stdout(io.StringIO()) as out:
with self.assertLogs(cmd.__name__, logging.CRITICAL) as ctx:
refresh()
self.assertRegex(out.getvalue(), r"\AWARNING: Bad git executable.\n")
self.assertEqual(len(ctx.records), 1)
message = ctx.records[0].getMessage()
self.assertRegex(message, r"\AWARNING: Bad git executable.\n")
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git")

@ddt.data(("2",), ("r",), ("raise",), ("e",), ("error",))
Expand Down

0 comments on commit ac50d8e

Please sign in to comment.