Commit da3460c
committed
Extract shared test logic to a helper
This also helps mock Popen over a smaller scope, which may be
beneficial (especially if it is mocked in the subprocess module,
rather than the git.cmd module, in the future).1 parent 0f19fb0 commit da3460c
1 file changed
Lines changed: 16 additions & 18 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -86,35 +86,33 @@ def test_it_transforms_kwargs_into_git_command_arguments(self): | |||
| 86 | 86 | (True, True, True), | |
| 87 | 87 | ) | |
| 88 | 88 | ||
| 89 | + def _do_shell_combo(self, value_in_call, value_from_class): | ||
| 90 | + with mock.patch.object(Git, "USE_SHELL", value_from_class): | ||
| 91 | + # git.cmd gets Popen via a "from" import, so patch it there. | ||
| 92 | + with mock.patch.object(cmd, "Popen", wraps=cmd.Popen) as mock_popen: | ||
| 93 | + # Use a command with no arguments (besides the program name), so it runs | ||
| 94 | + # with or without a shell, on all OSes, with the same effect. Since git | ||
| 95 | + # errors out when run with no arguments, we swallow that error. | ||
| 96 | + with contextlib.suppress(GitCommandError): | ||
| 97 | + self.git.execute(["git"], shell=value_in_call) | ||
| 98 | + | ||
| 99 | + return mock_popen | ||
| 100 | + | ||
| 89 | 101 | @ddt.idata(_shell_cases) | |
| 90 | - @mock.patch.object(cmd, "Popen", wraps=cmd.Popen) # Since it is gotten via a "from" import. | ||
| 91 | - def test_it_uses_shell_or_not_as_specified(self, case, mock_popen): | ||
| 102 | + def test_it_uses_shell_or_not_as_specified(self, case): | ||
| 92 | 103 | """A bool passed as ``shell=`` takes precedence over `Git.USE_SHELL`.""" | |
| 93 | 104 | value_in_call, value_from_class, expected_popen_arg = case | |
| 94 | - | ||
| 95 | - with mock.patch.object(Git, "USE_SHELL", value_from_class): | ||
| 96 | - with contextlib.suppress(GitCommandError): | ||
| 97 | - self.git.execute( | ||
| 98 | - ["git"], # No args, so it runs with or without a shell, on all OSes. | ||
| 99 | - shell=value_in_call, | ||
| 100 | - ) | ||
| 101 | - | ||
| 105 | + mock_popen = self._do_shell_combo(value_in_call, value_from_class) | ||
| 102 | 106 | mock_popen.assert_called_once() | |
| 103 | 107 | self.assertIs(mock_popen.call_args.kwargs["shell"], expected_popen_arg) | |
| 104 | 108 | ||
| 105 | 109 | @ddt.idata(full_case[:2] for full_case in _shell_cases) | |
| 106 | - @mock.patch.object(cmd, "Popen", wraps=cmd.Popen) # Since it is gotten via a "from" import. | ||
| 107 | - def test_it_logs_if_it_uses_a_shell(self, case, mock_popen): | ||
| 110 | + def test_it_logs_if_it_uses_a_shell(self, case): | ||
| 108 | 111 | """``shell=`` in the log message agrees with what is passed to `Popen`.""" | |
| 109 | 112 | value_in_call, value_from_class = case | |
| 110 | 113 | ||
| 111 | 114 | with self.assertLogs(cmd.log, level=logging.DEBUG) as log_watcher: | |
| 112 | - with mock.patch.object(Git, "USE_SHELL", value_from_class): | ||
| 113 | - with contextlib.suppress(GitCommandError): | ||
| 114 | - self.git.execute( | ||
| 115 | - ["git"], # No args, so it runs with or without a shell, on all OSes. | ||
| 116 | - shell=value_in_call, | ||
| 117 | - ) | ||
| 115 | + mock_popen = self._do_shell_combo(value_in_call, value_from_class) | ||
| 118 | 116 | ||
| 119 | 117 | popen_shell_arg = mock_popen.call_args.kwargs["shell"] | |
| 120 | 118 | expected_message = re.compile(rf"DEBUG:git.cmd:Popen\(.*\bshell={popen_shell_arg}\b.*\)") | |
0 commit comments