Skip to content

Clarify Git.execute and Popen arguments #1688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 3, 2023
Prev Previous commit
Next Next commit
Extract a _assert_logged_for_popen method
This extracts the logic of searching log messages, and asserting
that (at least) one matches a pattern for the report of a Popen
call with a given argument, from test_it_logs_if_it_uses_a_shell
into a new nonpublic test helper method _assert_logged_for_popen.

The extracted version is modified to make it slightly more general,
and slightly more robust. This is still not extremely robust: the
notation used to log Popen calls is informal, so it wouldn't make
sense to really parse it as code. But this no longer assumes that
the representation of a value ends at a word boundary, nor that the
value is free of regular expression metacharacters.
  • Loading branch information
EliahKagan committed Oct 3, 2023
commit 9fa1ceef2c47c9f58e10d8925cc166fdfd6b5183
14 changes: 8 additions & 6 deletions test/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ def tearDown(self):

gc.collect()

def _assert_logged_for_popen(self, log_watcher, name, value):
re_name = re.escape(name)
re_value = re.escape(str(value))
re_line = re.compile(fr"DEBUG:git.cmd:Popen\(.*\b{re_name}={re_value}[,)]")
match_attempts = [re_line.match(message) for message in log_watcher.output]
self.assertTrue(any(match_attempts), repr(log_watcher.output))

@mock.patch.object(Git, "execute")
def test_call_process_calls_execute(self, git):
git.return_value = ""
Expand Down Expand Up @@ -113,14 +120,9 @@ def test_it_uses_shell_or_not_as_specified(self, case):
def test_it_logs_if_it_uses_a_shell(self, case):
"""``shell=`` in the log message agrees with what is passed to `Popen`."""
value_in_call, value_from_class = case

with self.assertLogs(cmd.log, level=logging.DEBUG) as log_watcher:
mock_popen = self._do_shell_combo(value_in_call, value_from_class)

popen_shell_arg = mock_popen.call_args.kwargs["shell"]
expected_message = re.compile(rf"DEBUG:git.cmd:Popen\(.*\bshell={popen_shell_arg}\b.*\)")
match_attempts = [expected_message.fullmatch(message) for message in log_watcher.output]
self.assertTrue(any(match_attempts), repr(log_watcher.output))
self._assert_logged_for_popen(log_watcher, "shell", mock_popen.call_args.kwargs["shell"])

def test_it_executes_git_and_returns_result(self):
self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$")
Expand Down