Skip to content

Commit

Permalink
Further expand "not from cwd" test re: cmd.exe
Browse files Browse the repository at this point in the history
On Python versions for which python/cpython#101283 is not patched,
using Popen with shell=True can find cmd.exe in the current
directory on Windows, in the rare case that the ComSpec environment
variable is not defined.

This is not necessarily worth addressing, because it is a bug in
CPython rahter than GitPython, because that bug has been patched,
and because it is very rare that ComSpec is undefined. However:

- Changing the code to avoid it would also make that code simpler.
- Patched versions of Python <=3.9 don't have python.org builds.

This commit just expands the test to add cases where a repository
also has a file cmd.exe and where ComSpec is undefined, showing
that this case is not covered.
  • Loading branch information
EliahKagan committed Jan 9, 2024
1 parent 7da9c3b commit 865c6e8
Showing 1 changed file with 33 additions and 6 deletions.
39 changes: 33 additions & 6 deletions test/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,21 @@
from test.lib import TestBase, fixture_path, with_rw_directory


@contextlib.contextmanager
def _patch_out_env(name):
try:
old_value = os.environ[name]
except KeyError:
old_value = None
else:
del os.environ[name]
try:
yield
finally:
if old_value is not None:
os.environ[name] = old_value


@ddt.ddt
class TestGit(TestBase):
@classmethod
Expand Down Expand Up @@ -137,14 +152,17 @@ def test_it_executes_git_and_returns_result(self):
self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$")

@ddt.data(
(False, False, ["git", "version"]),
(False, True, "git version"),
(True, False, ["git", "version"]),
(True, True, "git version"),
# chdir_to_repo, shell, command, use_shell_impostor
(False, False, ["git", "version"], False),
(False, True, "git version", False),
(False, True, "git version", True),
(True, False, ["git", "version"], False),
(True, True, "git version", False),
(True, True, "git version", True),
)
@with_rw_directory
def test_it_executes_git_not_from_cwd(self, rw_dir, case):
chdir_to_repo, shell, command = case
chdir_to_repo, shell, command, use_shell_impostor = case

repo = Repo.init(rw_dir)

Expand All @@ -160,7 +178,16 @@ def test_it_executes_git_not_from_cwd(self, rw_dir, case):
impostor_path.write_text("#!/bin/sh\n", encoding="utf-8")
os.chmod(impostor_path, 0o755)

with cwd(rw_dir) if chdir_to_repo else contextlib.nullcontext():
if use_shell_impostor:
shell_name = "cmd.exe" if os.name == "nt" else "sh"
shutil.copy(impostor_path, Path(rw_dir, shell_name))

with contextlib.ExitStack() as stack:
if chdir_to_repo:
stack.enter_context(cwd(rw_dir))
if use_shell_impostor:
stack.enter_context(_patch_out_env("ComSpec"))

# Run the command without raising an exception on failure, as the exception
# message is currently misleading when the command is a string rather than a
# sequence of strings (it really runs "git", but then wrongly reports "g").
Expand Down

0 comments on commit 865c6e8

Please sign in to comment.