Skip to content

Commit

Permalink
Replace exp_sig with expected_returncode - closes #457
Browse files Browse the repository at this point in the history
  • Loading branch information
fizyk committed May 13, 2021
1 parent b3a01e9 commit abc154b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 21 deletions.
12 changes: 12 additions & 0 deletions CHANGES.rst
@@ -1,6 +1,18 @@
CHANGELOG
=========

unreleased
----------

Feauters
--------

- Replace `exp_sig` executor parameter with `expected_returncode`.
Parameter description already assumed that, however handing it assumed full
POSIX compatibility on the process side. Now the POSIX is only assumed if no
`expected_returncode` is passed to the executor, and returncode is simply that,
a returncode, nothing more

2.3.1
----------

Expand Down
36 changes: 17 additions & 19 deletions src/mirakuru/base.py
Expand Up @@ -101,7 +101,7 @@ def __init__( # pylint:disable=too-many-arguments
sleep: float = 0.1,
stop_signal: int = signal.SIGTERM,
kill_signal: int = SIGKILL,
exp_sig: int = None,
expected_returncode: int = None,
envvars: Optional[Dict[str, str]] = None,
stdin: Union[None, int, IO[Any]] = subprocess.PIPE,
stdout: Union[None, int, IO[Any]] = subprocess.PIPE,
Expand All @@ -121,8 +121,9 @@ def __init__( # pylint:disable=too-many-arguments
default is `signal.SIGTERM`
:param int kill_signal: signal used to kill process run by the executor.
default is `signal.SIGKILL` (`signal.SIGTERM` on Windows)
:param int exp_sig: expected exit code.
default is None as it will fall back to the stop signal
:param int expected_returncode: expected exit code.
default is None which means, Executor will determine a POSIX
compatible return code based on signal sent.
:param dict envvars: Additional environment variables
:param int stdin: file descriptor for stdin
:param int stdout: file descriptor for stdout
Expand Down Expand Up @@ -157,7 +158,7 @@ def __init__( # pylint:disable=too-many-arguments
self._sleep = sleep
self._stop_signal = stop_signal
self._kill_signal = kill_signal
self._expected_signal = exp_sig
self._expected_returncode = expected_returncode
self._envvars = envvars or {}

self._stdin = stdin
Expand Down Expand Up @@ -310,7 +311,7 @@ def _kill_all_kids(self, sig: int) -> Set[int]:
def stop(
self: SimpleExecutorType,
stop_signal: int = None,
expected_signal: int = None,
expected_returncode: int = None,
) -> SimpleExecutorType:
"""
Stop process running.
Expand All @@ -319,9 +320,9 @@ def stop(
:param int stop_signal: signal used to stop process run by executor.
None for default.
:param int expected_signal: expected exit code.
None for default.
:returns: itself
:param int expected_returncode: expected exit code.
None for default - POSIX compatible behaviour.
:returns: self
:rtype: SimpleExecutor
.. note::
Expand All @@ -335,9 +336,6 @@ def stop(
if stop_signal is None:
stop_signal = self._stop_signal

if expected_signal is None:
expected_signal = self._expected_signal

try:
os.killpg(self.process.pid, stop_signal)
except OSError as err:
Expand Down Expand Up @@ -365,15 +363,15 @@ def process_stopped() -> bool:
exit_code = self.process.wait()
self._clear_process()

# Did the process shut down cleanly? A an exit code of `-stop_signal`
# means that it has terminated due to signal `stop_signal`,
# which is intended. So don't treat that as an error.
# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.returncode
expected_exit_code = -stop_signal
if expected_signal is not None:
expected_exit_code = -expected_signal
if expected_returncode is None:
expected_returncode = self._expected_returncode
if expected_returncode is None:
# Assume a POSIX approach where sending a SIGNAL means
# that the process should exist with -SIGNAL exit code.
# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.returncode
expected_returncode = -stop_signal

if exit_code and exit_code != expected_exit_code:
if exit_code and exit_code != expected_returncode:
raise ProcessFinishedWithError(self, exit_code)

return self
Expand Down
10 changes: 8 additions & 2 deletions tests/executors/test_executor.py
Expand Up @@ -65,13 +65,19 @@ def test_stop_custom_exit_signal_stop():
executor = SimpleExecutor("false", shell=True)
executor.start()
# false exits instant, so there should not be a process to stop
retry(lambda: executor.stop(stop_signal=signal.SIGQUIT, expected_signal=3))
retry(
lambda: executor.stop(
stop_signal=signal.SIGQUIT, expected_returncode=-3
)
)
assert executor.running() is False


def test_stop_custom_exit_signal_context():
"""Start process and expect custom exit signal in context manager."""
with SimpleExecutor("false", exp_sig=3, shell=True) as executor:
with SimpleExecutor(
"false", expected_returncode=-3, shell=True
) as executor:
executor.stop(stop_signal=signal.SIGQUIT)
assert executor.running() is False

Expand Down

0 comments on commit abc154b

Please sign in to comment.