Report actual attempted Git command when Git.refresh fails by EliahKagan · Pull Request #1812 · gitpython-developers/GitPython

added 10 commits

January 24, 2024 13:38
For when refreshing should succeed and when it should fail.
shutil.which does not always find the git that Popen would run, at
least on Windows, but it is no worse than running a "command -v" or
"where" command as was previously done here via os.popen.
This extends test_refresh_bad_git_path so that it asserts that the
exception message shows the command the failed refresh used.

This test fails due to a bug where "git" is always shown (gitpython-developers#1809).
+ Test a successful refresh with a relative path, which will be
  safer to do once the refresh tests restore changed state.

See gitpython-developers#1811. This addresses it incompletely, because while it is
probably not necessary while running the test suite to preserve an
old False value of git.GIT_OK (most tests can't work if that
happens anyway), the value of Git.GIT_PYTHON_GIT_EXECUTABLE is not
the only other global state that effects the behavior of
subsequently run tests and that may be changed as a result of the
refresh tests.

1. After the git.refresh function calls git.cmd.Git.refresh, it
   calls git.remote.FetchInfo.refresh, which rebinds the
   git.remote.FetchInfo._flag_map attribute.

2. Future changes to git.cmd.Git.refresh may mutate other state in
   the Git class, and ideally the coupling would be loose enough
   that the refresh tests wouldn't have to be updated for that if
   the behavior being tested does not change.

3. Future changes to git.refresh may perform other refreshing
   actions, and ideally it would be easy (and obvious) what has to
   be done to patch it back. In particular, it will likely call
   another Git method that mutates class-wide state due to gitpython-developers#1791,
   and for such state that is also of the Git class, ideally no
   further changes would have to be made to the code that restores
   state after the refresh tests.

If we assume git.refresh is working at least in the case that it is
called with no arguments, then the cleanup can just be a call to
git.refresh(). Otherwise, sufficiently general cleanup may be more
complicated.
For gitpython-developers#1811. This is the simple way to do it. This relies on
git.refresh working when called when no arguments, so if that ever
breaks, then it may be difficult or inefficient to investigate. But
this is simpler than any alternatives that are equally or more
robust.

This is already better than the previous incomplete way that only
restored Git.GIT_PYTHON_GIT_EXECUTABLE (and nothing in FetchInfo).
Furthermore, because the tests already depend to a large extent on
git.refreh working when called with no arguments, as otherwise the
initial refresh may not have set Git.GIT_PYTHON_GIT_EXECUTABLE
usably, it might not be worthwhile to explore other approaches.
I think this refactoring slightly improves readability.

+ Condense/sort/group imports (while adding contextlib import).

+ Minor style tweaks in FetchInfo.refresh comments.

+ Make a comment in Git.refresh clearer, building on revisions in:
  gitpython-developers#1810 (comment)
This adds short docstrings to each of the four refresh tests to
briefly state the claims they verify.

lettuce-bot bot referenced this pull request in lettuce-financial/github-bot-signed-commit

Feb 15, 2024

renovate bot referenced this pull request in allenporter/flux-local

Feb 16, 2024