FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Merge pull request #1815 from EliahKagan/refresh-env · gitpython-developers/GitPython@db6fb90 · GitHub

/ GitPython Public

Commit db6fb90

Browse files
Byron
authored
Merge pull request #1815 from EliahKagan/refresh-env
Have initial refresh use a logger to warn
2 parents f4ce709 + 3a6e3ef commit db6fb90

2 files changed

Lines changed: 152 additions & 17 deletions

File tree

‎git/cmd.py‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -409,15 +409,15 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool:
409409
# expect GIT_PYTHON_REFRESH to either be unset or be one of the
410410
# following values:
411411
#
412-
# 0|q|quiet|s|silence|n|none
413-
# 1|w|warn|warning
414-
# 2|r|raise|e|error
412+
# 0|q|quiet|s|silence|silent|n|none
413+
# 1|w|warn|warning|l|log
414+
# 2|r|raise|e|error|exception
415415

416416
mode = os.environ.get(cls._refresh_env_var, "raise").lower()
417417

418-
quiet = ["quiet", "q", "silence", "s", "none", "n", "0"]
419-
warn = ["warn", "w", "warning", "1"]
420-
error = ["error", "e", "raise", "r", "2"]
418+
quiet = ["quiet", "q", "silence", "s", "silent", "none", "n", "0"]
419+
warn = ["warn", "w", "warning", "log", "l", "1"]
420+
error = ["error", "e", "exception", "raise", "r", "2"]
421421

422422
if mode in quiet:
423423
pass
@@ -428,10 +428,10 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool:
428428
%s
429429
All git commands will error until this is rectified.
430430
431-
This initial warning can be silenced or aggravated in the future by setting the
431+
This initial message can be silenced or aggravated in the future by setting the
432432
$%s environment variable. Use one of the following values:
433-
- %s: for no warning or exception
434-
- %s: for a printed warning
433+
- %s: for no message or exception
434+
- %s: for a warning message (logged at level CRITICAL, displayed by default)
435435
- %s: for a raised exception
436436
437437
Example:
@@ -450,7 +450,7 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool:
450450
)
451451

452452
if mode in warn:
453-
print("WARNING: %s" % err)
453+
_logger.critical("WARNING: %s", err)
454454
else:
455455
raise ImportError(err)
456456
else:
@@ -460,8 +460,8 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool:
460460
%s environment variable has been set but it has been set with an invalid value.
461461
462462
Use only the following values:
463-
- %s: for no warning or exception
464-
- %s: for a printed warning
463+
- %s: for no message or exception
464+
- %s: for a warning message (logged at level CRITICAL, displayed by default)
465465
- %s: for a raised exception
466466
"""
467467
)

‎test/test_git.py‎

Lines changed: 140 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,24 @@ def _patch_out_env(name):
4646

4747
@contextlib.contextmanager
4848
def _rollback_refresh():
49+
old_git_executable = Git.GIT_PYTHON_GIT_EXECUTABLE
50+
51+
if old_git_executable is None:
52+
raise RuntimeError("no executable string (need initial refresh before test)")
53+
4954
try:
50-
yield Git.GIT_PYTHON_GIT_EXECUTABLE # Provide the old value for convenience.
55+
yield old_git_executable # Provide the old value for convenience.
5156
finally:
57+
# The cleanup refresh should always raise an exception if it fails, since if it
58+
# fails then previously discovered test results could be misleading and, more
59+
# importantly, subsequent tests may be unable to run or give misleading results.
60+
# So pre-set a non-None value, so that the cleanup will be a "second" refresh.
61+
# This covers cases where a test has set it to None to test a "first" refresh.
62+
Git.GIT_PYTHON_GIT_EXECUTABLE = Git.git_exec_name
63+
64+
# Do the cleanup refresh. This sets Git.GIT_PYTHON_GIT_EXECUTABLE to old_value
65+
# in most cases. The reason to call it is to achieve other associated state
66+
# changes as well, which include updating attributes of the FetchInfo class.
5267
refresh()
5368

5469

@@ -314,7 +329,127 @@ def test_cmd_override(self):
314329
):
315330
self.assertRaises(GitCommandNotFound, self.git.version)
316331

317-
def test_refresh_bad_absolute_git_path(self):
332+
def test_git_exc_name_is_git(self):
333+
self.assertEqual(self.git.git_exec_name, "git")
334+
335+
@ddt.data(("0",), ("q",), ("quiet",), ("s",), ("silence",), ("silent",), ("n",), ("none",))
336+
def test_initial_refresh_from_bad_git_path_env_quiet(self, case):
337+
"""In "q" mode, bad initial path sets "git" and is quiet."""
338+
(mode,) = case
339+
set_vars = {
340+
"GIT_PYTHON_GIT_EXECUTABLE": str(Path("yada").absolute()), # Any bad path.
341+
"GIT_PYTHON_REFRESH": mode,
342+
}
343+
with _rollback_refresh():
344+
type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup.
345+
346+
with mock.patch.dict(os.environ, set_vars):
347+
refresh()
348+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git")
349+
350+
@ddt.data(("1",), ("w",), ("warn",), ("warning",), ("l",), ("log",))
351+
def test_initial_refresh_from_bad_git_path_env_warn(self, case):
352+
"""In "w" mode, bad initial path sets "git" and warns, by logging."""
353+
(mode,) = case
354+
env_vars = {
355+
"GIT_PYTHON_GIT_EXECUTABLE": str(Path("yada").absolute()), # Any bad path.
356+
"GIT_PYTHON_REFRESH": mode,
357+
}
358+
with _rollback_refresh():
359+
type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup.
360+
361+
with mock.patch.dict(os.environ, env_vars):
362+
with self.assertLogs(cmd.__name__, logging.CRITICAL) as ctx:
363+
refresh()
364+
self.assertEqual(len(ctx.records), 1)
365+
message = ctx.records[0].getMessage()
366+
self.assertRegex(message, r"\AWARNING: Bad git executable.\n")
367+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git")
368+
369+
@ddt.data(("2",), ("r",), ("raise",), ("e",), ("error",))
370+
def test_initial_refresh_from_bad_git_path_env_error(self, case):
371+
"""In "e" mode, bad initial path raises an exception."""
372+
(mode,) = case
373+
env_vars = {
374+
"GIT_PYTHON_GIT_EXECUTABLE": str(Path("yada").absolute()), # Any bad path.
375+
"GIT_PYTHON_REFRESH": mode,
376+
}
377+
with _rollback_refresh():
378+
type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup.
379+
380+
with mock.patch.dict(os.environ, env_vars):
381+
with self.assertRaisesRegex(ImportError, r"\ABad git executable.\n"):
382+
refresh()
383+
384+
def test_initial_refresh_from_good_absolute_git_path_env(self):
385+
"""Good initial absolute path from environment is set."""
386+
absolute_path = shutil.which("git")
387+
388+
with _rollback_refresh():
389+
type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup.
390+
391+
with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": absolute_path}):
392+
refresh()
393+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, absolute_path)
394+
395+
def test_initial_refresh_from_good_relative_git_path_env(self):
396+
"""Good initial relative path from environment is kept relative and set."""
397+
with _rollback_refresh():
398+
# Set the fallback to a string that wouldn't work and isn't "git", so we are
399+
# more likely to detect if "git" is not set from the environment variable.
400+
with mock.patch.object(type(self.git), "git_exec_name", ""):
401+
type(self.git).GIT_PYTHON_GIT_EXECUTABLE = None # Simulate startup.
402+
403+
# Now observe if setting the environment variable to "git" takes effect.
404+
with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": "git"}):
405+
refresh()
406+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git")
407+
408+
def test_refresh_from_bad_absolute_git_path_env(self):
409+
"""Bad absolute path from environment is reported and not set."""
410+
absolute_path = str(Path("yada").absolute())
411+
expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z"
412+
413+
with _rollback_refresh() as old_git_executable:
414+
with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": absolute_path}):
415+
with self.assertRaisesRegex(GitCommandNotFound, expected_pattern):
416+
refresh()
417+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable)
418+
419+
def test_refresh_from_bad_relative_git_path_env(self):
420+
"""Bad relative path from environment is kept relative and reported, not set."""
421+
# Relative paths are not resolved when refresh() is called with no arguments, so
422+
# use a string that's very unlikely to be a command name found in a path lookup.
423+
relative_path = "yada-e47e70c6-acbf-40f8-ad65-13af93c2195b"
424+
expected_pattern = rf"\n[ \t]*cmdline: {re.escape(relative_path)}\Z"
425+
426+
with _rollback_refresh() as old_git_executable:
427+
with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": relative_path}):
428+
with self.assertRaisesRegex(GitCommandNotFound, expected_pattern):
429+
refresh()
430+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable)
431+
432+
def test_refresh_from_good_absolute_git_path_env(self):
433+
"""Good absolute path from environment is set."""
434+
absolute_path = shutil.which("git")
435+
436+
with _rollback_refresh():
437+
with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": absolute_path}):
438+
refresh()
439+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, absolute_path)
440+
441+
def test_refresh_from_good_relative_git_path_env(self):
442+
"""Good relative path from environment is kept relative and set."""
443+
with _rollback_refresh():
444+
# Set as the executable name a string that wouldn't work and isn't "git".
445+
type(self.git).GIT_PYTHON_GIT_EXECUTABLE = ""
446+
447+
# Now observe if setting the environment variable to "git" takes effect.
448+
with mock.patch.dict(os.environ, {"GIT_PYTHON_GIT_EXECUTABLE": "git"}):
449+
refresh()
450+
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, "git")
451+
452+
def test_refresh_with_bad_absolute_git_path_arg(self):
318453
"""Bad absolute path arg is reported and not set."""
319454
absolute_path = str(Path("yada").absolute())
320455
expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z"
@@ -324,7 +459,7 @@ def test_refresh_bad_absolute_git_path(self):
324459
refresh(absolute_path)
325460
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable)
326461

327-
def test_refresh_bad_relative_git_path(self):
462+
def test_refresh_with_bad_relative_git_path_arg(self):
328463
"""Bad relative path arg is resolved to absolute path and reported, not set."""
329464
absolute_path = str(Path("yada").absolute())
330465
expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z"
@@ -334,15 +469,15 @@ def test_refresh_bad_relative_git_path(self):
334469
refresh("yada")
335470
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable)
336471

337-
def test_refresh_good_absolute_git_path(self):
472+
def test_refresh_with_good_absolute_git_path_arg(self):
338473
"""Good absolute path arg is set."""
339474
absolute_path = shutil.which("git")
340475

341476
with _rollback_refresh():
342477
refresh(absolute_path)
343478
self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, absolute_path)
344479

345-
def test_refresh_good_relative_git_path(self):
480+
def test_refresh_with_good_relative_git_path_arg(self):
346481
"""Good relative path arg is resolved to absolute path and set."""
347482
absolute_path = shutil.which("git")
348483
dirname, basename = osp.split(absolute_path)

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL