From aa5076626ca9f2ff1279c6b8e67408be9d0fa690 Mon Sep 17 00:00:00 2001 From: sroet Date: Wed, 15 Sep 2021 11:55:17 +0200 Subject: Add a way to force status codes inside AutoInterrupt._terminate, and let tests use it --- git/cmd.py | 19 ++++++++++++------- test/test_remote.py | 14 ++++++++++---- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 9279bb0c..8fb10742 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -409,6 +409,10 @@ class Git(LazyMixin): __slots__ = ("proc", "args", "status") + # If this is non-zero it will override any status code during + # _terminate, used to prevent race conditions in testing + _status_code_if_terminate: int = 0 + def __init__(self, proc: Union[None, subprocess.Popen], args: Any) -> None: self.proc = proc self.args = args @@ -427,11 +431,10 @@ class Git(LazyMixin): proc.stdout.close() if proc.stderr: proc.stderr.close() - # did the process finish already so we have a return code ? try: if proc.poll() is not None: - self.status = proc.poll() + self.status = self._status_code_if_terminate or proc.poll() return None except OSError as ex: log.info("Ignored error after process had died: %r", ex) @@ -443,7 +446,9 @@ class Git(LazyMixin): # try to kill it try: proc.terminate() - self.status = proc.wait() # ensure process goes away + status = proc.wait() # ensure process goes away + + self.status = self._status_code_if_terminate or status except OSError as ex: log.info("Ignored error after process had died: %r", ex) except AttributeError: @@ -849,7 +854,7 @@ class Git(LazyMixin): if is_win: cmd_not_found_exception = OSError - if kill_after_timeout: + if kill_after_timeout is not None: raise GitCommandError(redacted_command, '"kill_after_timeout" feature is not supported on Windows.') else: cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable @@ -916,7 +921,7 @@ class Git(LazyMixin): return # end - if kill_after_timeout: + if kill_after_timeout is not None: kill_check = threading.Event() watchdog = threading.Timer(kill_after_timeout, _kill_process, args=(proc.pid,)) @@ -927,10 +932,10 @@ class Git(LazyMixin): newline = "\n" if universal_newlines else b"\n" try: if output_stream is None: - if kill_after_timeout: + if kill_after_timeout is not None: watchdog.start() stdout_value, stderr_value = proc.communicate() - if kill_after_timeout: + if kill_after_timeout is not None: watchdog.cancel() if kill_check.is_set(): stderr_value = ('Timeout: the command "%s" did not complete in %d ' diff --git a/test/test_remote.py b/test/test_remote.py index 4c1d02c8..088fdad5 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -658,10 +658,16 @@ class TestRemote(TestBase): class TestTimeouts(TestBase): @with_rw_repo('HEAD', bare=False) def test_timeout_funcs(self, repo): - for function in ["pull"]: # can't get fetch and push to reliably timeout + # Force error code to prevent a race condition if the python thread is + # slow + default = Git.AutoInterrupt._status_code_if_terminate + Git.AutoInterrupt._status_code_if_terminate = -15 + for function in ["pull", "fetch"]: # can't get push to timeout f = getattr(repo.remotes.origin, function) assert f is not None # Make sure these functions exist - _ = f() # Make sure the function runs + _ = f() # Make sure the function runs with pytest.raises(GitCommandError, - match="kill_after_timeout=0.001 s"): - f(kill_after_timeout=0.001) + match="kill_after_timeout=0 s"): + f(kill_after_timeout=0) + + Git.AutoInterrupt._status_code_if_terminate = default -- cgit v1.2.1