From 4bcd1cfb62f1884aa1d140aaa0f44e5324722177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Thu, 6 May 2021 14:40:33 +0200 Subject: [PATCH 1/9] Windows support --- .github/workflows/tests-windows.yml | 47 +++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .github/workflows/tests-windows.yml diff --git a/.github/workflows/tests-windows.yml b/.github/workflows/tests-windows.yml new file mode 100644 index 00000000..ab9d8255 --- /dev/null +++ b/.github/workflows/tests-windows.yml @@ -0,0 +1,47 @@ +name: Run tests on Windows + +on: + push: + branches: [ master ] + paths: + - '**.py' + - .github/workflows/tests-windows.yml + - requirements-test.txt + pull_request: + branches: [ master ] + paths: + - '**.py' + - .github/workflows/tests-windows.yml + - requirements-test.txt + +jobs: + windowstests: + runs-on: windows-latest + strategy: + fail-fast: false + matrix: + python-version: [3.8, 3.9] + env: + OS: windows-latest + PYTHON: ${{ matrix.python-version }} + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-test.txt + - name: Run test + run: | + py.test --cov-report=xml + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v1 + with: + file: ./coverage.xml + flags: windows + env_vars: OS, PYTHON, POSTGRES + fail_ci_if_error: true From dda60b5532cc44edc11cf6987d891492e0220475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Thu, 6 May 2021 16:36:47 +0200 Subject: [PATCH 2/9] Add killpg based on psutil --- src/mirakuru/base.py | 5 +- src/mirakuru/kill.py | 21 ++++++ tests/executors/test_executor.py | 83 ++++++++++++++++++++- tests/executors/test_executor_kill.py | 22 +++++- tests/executors/test_http_executor.py | 28 +++++++ tests/executors/test_output_executor.py | 12 +++ tests/executors/test_pid_executor.py | 16 ++++ tests/executors/test_tcp_executor.py | 12 +++ tests/executors/test_unixsocket_executor.py | 6 ++ 9 files changed, 200 insertions(+), 5 deletions(-) create mode 100644 src/mirakuru/kill.py diff --git a/src/mirakuru/base.py b/src/mirakuru/base.py index 49b40cfb..e9995a31 100644 --- a/src/mirakuru/base.py +++ b/src/mirakuru/base.py @@ -52,6 +52,7 @@ TimeoutExpired, ) from mirakuru.compat import SIGKILL +from mirakuru.kill import killpg LOG = logging.getLogger(__name__) @@ -337,7 +338,7 @@ def stop( stop_signal = self._stop_signal try: - os.killpg(self.process.pid, stop_signal) + killpg(self.process.pid, stop_signal) except OSError as err: if err.errno in IGNORED_ERROR_CODES: pass @@ -407,7 +408,7 @@ def kill( if sig is None: sig = self._kill_signal if self.process and self.running(): - os.killpg(self.process.pid, sig) + killpg(self.process.pid, sig) if wait: self.process.wait() diff --git a/src/mirakuru/kill.py b/src/mirakuru/kill.py new file mode 100644 index 00000000..0c3020fb --- /dev/null +++ b/src/mirakuru/kill.py @@ -0,0 +1,21 @@ +import os +from typing import List + +try: + import psutil +except ImportError: + psutil = None + +killpg = getattr(os, "killpg", None) + +if not killpg: + if psutil: + + def killpg(pid, sig): + process = psutil.Process(pid) + children: List[psutil.Process] = process.children(recursive=True) + for child in children: + child.send_signal(sig) + psutil.wait_procs(children, timeout=30) + process.send_signal(sig) + process.wait(timeout=30) diff --git a/tests/executors/test_executor.py b/tests/executors/test_executor.py index ddd2722c..9bfa09de 100644 --- a/tests/executors/test_executor.py +++ b/tests/executors/test_executor.py @@ -19,6 +19,9 @@ SLEEP_300 = "sleep 300" +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="Expects signal -15 gets 15" +) @pytest.mark.parametrize("command", (SLEEP_300, SLEEP_300.split())) def test_running_process(command): """Start process and shuts it down.""" @@ -33,6 +36,9 @@ def test_running_process(command): assert SLEEP_300 in str(executor) +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="Expects signal -15 gets 15" +) @pytest.mark.parametrize("command", (SLEEP_300, SLEEP_300.split())) def test_command(command): """Check that the command and command parts are equivalent.""" @@ -42,6 +48,9 @@ def test_command(command): assert executor.command_parts == SLEEP_300.split() +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_custom_signal_stop(): """Start process and shuts it down using signal SIGQUIT.""" executor = SimpleExecutor(SLEEP_300, stop_signal=signal.SIGQUIT) @@ -51,6 +60,9 @@ def test_custom_signal_stop(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_stop_custom_signal_stop(): """Start process and shuts it down using signal SIGQUIT passed to stop.""" executor = SimpleExecutor(SLEEP_300) @@ -60,6 +72,9 @@ def test_stop_custom_signal_stop(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_stop_custom_exit_signal_stop(): """Start process and expect it to finish with custom signal.""" executor = SimpleExecutor("false", shell=True) @@ -73,6 +88,9 @@ def test_stop_custom_exit_signal_stop(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_stop_custom_exit_signal_context(): """Start process and expect custom exit signal in context manager.""" with SimpleExecutor( @@ -82,6 +100,9 @@ def test_stop_custom_exit_signal_context(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_running_context(): """Start process and shuts it down.""" executor = SimpleExecutor(SLEEP_300) @@ -91,12 +112,18 @@ def test_running_context(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_executor_in_context_only(): """Start process and shuts it down only in context.""" with SimpleExecutor(SLEEP_300) as executor: assert executor.running() is True +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_context_stopped(): """Start for context, and shuts it for nested context.""" executor = SimpleExecutor(SLEEP_300) @@ -112,7 +139,28 @@ def test_context_stopped(): ECHO_FOOBAR = 'echo "foobar"' -@pytest.mark.parametrize("command", (ECHO_FOOBAR, shlex.split(ECHO_FOOBAR))) +@pytest.mark.parametrize( + "command", + ( + pytest.param( + ECHO_FOOBAR, + marks=pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Windows leaves apostrophes in output.", + ), + ), + pytest.param( + shlex.split(ECHO_FOOBAR), + marks=pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=( + "No such process when stopping. It's Echo, so at the moment " + "of psutil.Process creation to kill it, it's already stopped." + ), + ), + ), + ), +) def test_process_output(command): """Start process, check output and shut it down.""" executor = SimpleExecutor(command) @@ -122,7 +170,28 @@ def test_process_output(command): executor.stop() -@pytest.mark.parametrize("command", (ECHO_FOOBAR, shlex.split(ECHO_FOOBAR))) +@pytest.mark.parametrize( + "command", + ( + pytest.param( + ECHO_FOOBAR, + marks=pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Windows leaves apostrophes in output.", + ), + ), + pytest.param( + shlex.split(ECHO_FOOBAR), + marks=pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=( + "No such process when stopping. It's Echo, so at the moment " + "of psutil.Process creation to kill it, it's already stopped." + ), + ), + ), + ), +) def test_process_output_shell(command): """Start process, check output and shut it down with shell set to True.""" executor = SimpleExecutor(command, shell=True) @@ -141,6 +210,10 @@ def test_start_check_executor(): executor.after_start_check() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Expects signal -15 gets 15 at the last stop", +) def test_stopping_not_yet_running_executor(): """ Test if SimpleExecutor can be stopped even it was never running. @@ -155,6 +228,7 @@ def test_stopping_not_yet_running_executor(): executor.stop() +@pytest.mark.skipif("platform.system() == 'Windows'", reason="No ps_uax") def test_forgotten_stop(): """ Test if SimpleExecutor subprocess is killed after an instance is deleted. @@ -236,6 +310,10 @@ def test_executor_ignores_processes_exiting_with_0(): assert executor.after_start_check.called is True # type: ignore +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Expects signal -15 gets 15 at the last stop", +) def test_executor_methods_returning_self(): """Test if SimpleExecutor lets to chain start, stop and kill methods.""" executor = SimpleExecutor(SLEEP_300).start().stop().kill().stop() @@ -251,6 +329,7 @@ def test_executor_methods_returning_self(): assert SimpleExecutor(SLEEP_300).start().stop().output +@pytest.mark.skipif("platform.system() == 'Windows'", reason="No ps_uax") def test_mirakuru_cleanup(): """Test if cleanup_subprocesses is fired correctly on python exit.""" cmd = f""" diff --git a/tests/executors/test_executor_kill.py b/tests/executors/test_executor_kill.py index 8c9377de..c09a13d4 100644 --- a/tests/executors/test_executor_kill.py +++ b/tests/executors/test_executor_kill.py @@ -9,17 +9,22 @@ import os from unittest.mock import patch +import psutil import pytest from mirakuru import SimpleExecutor, HTTPExecutor from mirakuru.compat import SIGKILL from mirakuru.exceptions import ProcessFinishedWithError +from mirakuru.kill import killpg from tests import SAMPLE_DAEMON_PATH, ps_aux, TEST_SERVER_PATH SLEEP_300 = "sleep 300" +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_custom_signal_kill(): """Start process and shuts it down using signal SIGQUIT.""" executor = SimpleExecutor(SLEEP_300, kill_signal=signal.SIGQUIT) @@ -29,6 +34,9 @@ def test_custom_signal_kill(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_kill_custom_signal_kill(): """Start process and shuts it down using signal SIGQUIT passed to kill.""" executor = SimpleExecutor(SLEEP_300) @@ -38,12 +46,19 @@ def test_kill_custom_signal_kill(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=( + "No such process when stopping. It's Echo, so at the moment " + "of psutil.Process creation to kill it, it's already stopped." + ), +) def test_already_closed(): """Check that the executor cleans after itself after it exited earlier.""" with pytest.raises(ProcessFinishedWithError) as excinfo: with SimpleExecutor("python") as executor: assert executor.running() - os.killpg(executor.process.pid, SIGKILL) + killpg(executor.process.pid, SIGKILL) def process_stopped(): """Return True only only when self.process is not running.""" @@ -55,6 +70,7 @@ def process_stopped(): assert not executor.process +@pytest.mark.skipif("platform.system() == 'Windows'", reason="No ps_uax") def test_daemons_killing(): """ Test if all subprocesses of SimpleExecutor can be killed. @@ -75,6 +91,10 @@ def test_daemons_killing(): assert SAMPLE_DAEMON_PATH not in ps_aux() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Expects signal -15 gets 15 at the last stop", +) def test_stopping_brutally(): """ Test if SimpleExecutor is stopping insubordinate process. diff --git a/tests/executors/test_http_executor.py b/tests/executors/test_http_executor.py index df8dcdcc..36116cfd 100644 --- a/tests/executors/test_http_executor.py +++ b/tests/executors/test_http_executor.py @@ -34,6 +34,10 @@ def connect_to_server(): conn.close() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Can't start http.server on python3", +) def test_executor_starts_and_waits(): """Test if process awaits for HEAD request to be completed.""" command = f'bash -c "sleep 3 && {HTTP_NORMAL_CMD}"' @@ -51,6 +55,10 @@ def test_executor_starts_and_waits(): assert command in str(executor) +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Expects signal -15 gets 15 at the last stop", +) def test_shell_started_server_stops(): """Test if executor terminates properly executor with shell=True.""" executor = HTTPExecutor( @@ -70,6 +78,10 @@ def test_shell_started_server_stops(): connect_to_server() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Expects signal -15 gets 15 at the last stop", +) @pytest.mark.parametrize("method", ("HEAD", "GET", "POST")) def test_slow_method_server_starting(method): """ @@ -92,6 +104,10 @@ def test_slow_method_server_starting(method): connect_to_server() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Expects signal -15 gets 15 at the last stop", +) def test_slow_post_payload_server_starting(): """ Test whether or not executor awaits for slow starting servers. @@ -114,6 +130,9 @@ def test_slow_post_payload_server_starting(): connect_to_server() +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No such process when stopping." +) @pytest.mark.parametrize("method", ("HEAD", "GET", "POST")) def test_slow_method_server_timed_out(method): """Check if timeout properly expires.""" @@ -132,6 +151,9 @@ def test_slow_method_server_timed_out(method): assert "timed out after" in str(exc.value) +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No such process when stopping." +) def test_fail_if_other_running(): """Test raising AlreadyRunning exception when port is blocked.""" executor = HTTPExecutor( @@ -156,6 +178,9 @@ def test_fail_if_other_running(): assert "seems to be already running" in str(exc.value) +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No such process when stopping." +) @patch.object(HTTPExecutor, "DEFAULT_PORT", PORT) def test_default_port(): """ @@ -175,6 +200,9 @@ def test_default_port(): executor.stop() +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No such process when stopping." +) @pytest.mark.parametrize( "accepted_status, expected_timeout", ( diff --git a/tests/executors/test_output_executor.py b/tests/executors/test_output_executor.py index 0c22837e..cef7e2ba 100644 --- a/tests/executors/test_output_executor.py +++ b/tests/executors/test_output_executor.py @@ -8,6 +8,10 @@ from mirakuru.exceptions import TimeoutExpired +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_executor_waits_for_process_output(): """Check if executor waits for specified output.""" command = 'bash -c "sleep 2 && echo foo && echo bar && sleep 100"' @@ -23,6 +27,10 @@ def test_executor_waits_for_process_output(): assert "foo" in str(executor) +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_executor_waits_for_process_err_output(): """Check if executor waits for specified error output.""" command = 'bash -c "sleep 2 && >&2 echo foo && >&2 echo bar && sleep 100"' @@ -40,6 +48,10 @@ def test_executor_waits_for_process_err_output(): assert "foo" in str(executor) +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_executor_dont_start(): """Executor should not start.""" command = 'bash -c "sleep 2 && echo foo && echo bar && sleep 100"' diff --git a/tests/executors/test_pid_executor.py b/tests/executors/test_pid_executor.py index 2db8dd3d..60e418bf 100644 --- a/tests/executors/test_pid_executor.py +++ b/tests/executors/test_pid_executor.py @@ -32,6 +32,10 @@ def run_around_tests(): pass +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_start_and_wait(): """Test if the executor will await for the process to create a file.""" process = f'bash -c "sleep 2 && touch {FILENAME} && sleep 10"' @@ -50,6 +54,10 @@ def test_empty_filename(pid_file): PidExecutor(SLEEP, pid_file) +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_if_file_created(): """Check whether the process really created the given file.""" assert os.path.isfile(FILENAME) is False @@ -58,6 +66,10 @@ def test_if_file_created(): assert os.path.isfile(FILENAME) is True +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_timeout_error(): """Check if timeout properly expires.""" executor = PidExecutor(SLEEP, FILENAME, timeout=1) @@ -68,6 +80,10 @@ def test_timeout_error(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_fail_if_other_executor_running(): """Test raising AlreadyRunning exception when port is blocked.""" process = f'bash -c "sleep 2 && touch {FILENAME} && sleep 10"' diff --git a/tests/executors/test_tcp_executor.py b/tests/executors/test_tcp_executor.py index ba72f482..3166e158 100644 --- a/tests/executors/test_tcp_executor.py +++ b/tests/executors/test_tcp_executor.py @@ -21,6 +21,10 @@ NC_COMMAND = 'bash -c "sleep 2 && nc -lk 3000"' +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_start_and_wait(caplog: LogCaptureFixture): """Test if executor await for process to accept connections.""" caplog.set_level(logging.DEBUG, logger="mirakuru") @@ -38,6 +42,10 @@ def test_repr_and_str(): assert NC_COMMAND in str(executor) +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_it_raises_error_on_timeout(): """Check if TimeoutExpired gets raised correctly.""" command = 'bash -c "sleep 10 && nc -lk 3000"' @@ -49,6 +57,10 @@ def test_it_raises_error_on_timeout(): assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_fail_if_other_executor_running(): """Test raising AlreadyRunning exception.""" executor = TCPExecutor(HTTP_SERVER, host="localhost", port=PORT) diff --git a/tests/executors/test_unixsocket_executor.py b/tests/executors/test_unixsocket_executor.py index 768e4536..79d5732b 100644 --- a/tests/executors/test_unixsocket_executor.py +++ b/tests/executors/test_unixsocket_executor.py @@ -17,6 +17,9 @@ SOCKET_SERVER_CMD = f"{sys.executable} {TEST_SOCKET_SERVER_PATH} {SOCKET_PATH}" +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="SIGQUIT not available on Windows" +) def test_start_and_wait(): """Test if executor await for process to accept connections.""" executor = UnixSocketExecutor( @@ -26,6 +29,9 @@ def test_start_and_wait(): assert executor.running() is True +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="SIGQUIT not available on Windows" +) def test_start_and_timeout(): """Test if executor will properly times out.""" executor = UnixSocketExecutor( From 189e0a746b275e391d6098d78fc3e393760e157d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Fri, 7 May 2021 17:26:37 +0200 Subject: [PATCH 3/9] Lint and tests --- src/mirakuru/kill.py | 2 +- tests/executors/test_executor.py | 10 ++++++---- tests/executors/test_unixsocket_executor.py | 6 ++++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/mirakuru/kill.py b/src/mirakuru/kill.py index 0c3020fb..aaa0c3d7 100644 --- a/src/mirakuru/kill.py +++ b/src/mirakuru/kill.py @@ -11,7 +11,7 @@ if not killpg: if psutil: - def killpg(pid, sig): + def killpg(pid: int, sig: int) -> None: process = psutil.Process(pid) children: List[psutil.Process] = process.children(recursive=True) for child in children: diff --git a/tests/executors/test_executor.py b/tests/executors/test_executor.py index 9bfa09de..1ee2858d 100644 --- a/tests/executors/test_executor.py +++ b/tests/executors/test_executor.py @@ -154,8 +154,9 @@ def test_context_stopped(): marks=pytest.mark.skipif( "platform.system() == 'Windows'", reason=( - "No such process when stopping. It's Echo, so at the moment " - "of psutil.Process creation to kill it, it's already stopped." + "No such process when stopping. It's Echo, " + "so at the moment of psutil.Process creation " + "to kill it, it's already stopped." ), ), ), @@ -185,8 +186,9 @@ def test_process_output(command): marks=pytest.mark.skipif( "platform.system() == 'Windows'", reason=( - "No such process when stopping. It's Echo, so at the moment " - "of psutil.Process creation to kill it, it's already stopped." + "No such process when stopping. It's Echo, " + "so at the moment of psutil.Process creation " + "to kill it, it's already stopped." ), ), ), diff --git a/tests/executors/test_unixsocket_executor.py b/tests/executors/test_unixsocket_executor.py index 79d5732b..a42eef3e 100644 --- a/tests/executors/test_unixsocket_executor.py +++ b/tests/executors/test_unixsocket_executor.py @@ -18,7 +18,8 @@ @pytest.mark.skipif( - "platform.system() == 'Windows'", reason="SIGQUIT not available on Windows" + "platform.system() == 'Windows'", + reason="Python does not support socket.AF_UNIX on windows", ) def test_start_and_wait(): """Test if executor await for process to accept connections.""" @@ -30,7 +31,8 @@ def test_start_and_wait(): @pytest.mark.skipif( - "platform.system() == 'Windows'", reason="SIGQUIT not available on Windows" + "platform.system() == 'Windows'", + reason="Python does not support socket.AF_UNIX on windows", ) def test_start_and_timeout(): """Test if executor will properly times out.""" From d2c1c870102d144bb7e5139d0a90529c1a28fdd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Thu, 13 May 2021 17:51:29 +0200 Subject: [PATCH 4/9] Windows is not POSIX compatible --- src/mirakuru/base.py | 15 +++++++++++---- src/mirakuru/compat.py | 10 ++++++++++ tests/executors/test_executor.py | 12 ++---------- tests/executors/test_executor_kill.py | 2 +- tests/executors/test_http_executor.py | 18 +++++++++++------- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/mirakuru/base.py b/src/mirakuru/base.py index e9995a31..8786404c 100644 --- a/src/mirakuru/base.py +++ b/src/mirakuru/base.py @@ -65,6 +65,9 @@ if platform.system() == "Darwin": IGNORED_ERROR_CODES = [errno.ESRCH, errno.EPERM] +IS_WINDOWS = platform.system() == "Windows" + + # Type variables used for self in functions returning self, so it's correctly # typed in derived classes. SimpleExecutorType = TypeVar("SimpleExecutorType", bound="SimpleExecutor") @@ -367,10 +370,14 @@ def process_stopped() -> bool: 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 IS_WINDOWS: + # Windows is not POSIX compatible + expected_returncode = stop_signal + else: + # 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_returncode: raise ProcessFinishedWithError(self, exit_code) diff --git a/src/mirakuru/compat.py b/src/mirakuru/compat.py index 1ad90e04..062060ab 100644 --- a/src/mirakuru/compat.py +++ b/src/mirakuru/compat.py @@ -16,10 +16,20 @@ # You should have received a copy of the GNU Lesser General Public License # along with mirakuru. If not, see . """Mirakuru compatibility module.""" +import platform import signal +IS_WINDOWS = platform.system() == "Windows" + # Windows does not have SIGKILL, fall back to SIGTERM. SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM) +def returncode_from_signal(signal: int) -> int: + """Calculates expected exit code based on signal. Used as a default behaviour in""" + if IS_WINDOWS: + return signal + return -signal + + __all__ = ("SIGKILL",) diff --git a/tests/executors/test_executor.py b/tests/executors/test_executor.py index 1ee2858d..f902b8af 100644 --- a/tests/executors/test_executor.py +++ b/tests/executors/test_executor.py @@ -154,7 +154,7 @@ def test_context_stopped(): marks=pytest.mark.skipif( "platform.system() == 'Windows'", reason=( - "No such process when stopping. It's Echo, " + "psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists. It's Echo, " "so at the moment of psutil.Process creation " "to kill it, it's already stopped." ), @@ -186,7 +186,7 @@ def test_process_output(command): marks=pytest.mark.skipif( "platform.system() == 'Windows'", reason=( - "No such process when stopping. It's Echo, " + "psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists. It's Echo, " "so at the moment of psutil.Process creation " "to kill it, it's already stopped." ), @@ -212,10 +212,6 @@ def test_start_check_executor(): executor.after_start_check() -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="Expects signal -15 gets 15 at the last stop", -) def test_stopping_not_yet_running_executor(): """ Test if SimpleExecutor can be stopped even it was never running. @@ -312,10 +308,6 @@ def test_executor_ignores_processes_exiting_with_0(): assert executor.after_start_check.called is True # type: ignore -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="Expects signal -15 gets 15 at the last stop", -) def test_executor_methods_returning_self(): """Test if SimpleExecutor lets to chain start, stop and kill methods.""" executor = SimpleExecutor(SLEEP_300).start().stop().kill().stop() diff --git a/tests/executors/test_executor_kill.py b/tests/executors/test_executor_kill.py index c09a13d4..8abd4a3a 100644 --- a/tests/executors/test_executor_kill.py +++ b/tests/executors/test_executor_kill.py @@ -49,7 +49,7 @@ def test_kill_custom_signal_kill(): @pytest.mark.skipif( "platform.system() == 'Windows'", reason=( - "No such process when stopping. It's Echo, so at the moment " + "psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists. It's Echo, so at the moment " "of psutil.Process creation to kill it, it's already stopped." ), ) diff --git a/tests/executors/test_http_executor.py b/tests/executors/test_http_executor.py index 36116cfd..88794727 100644 --- a/tests/executors/test_http_executor.py +++ b/tests/executors/test_http_executor.py @@ -57,7 +57,7 @@ def test_executor_starts_and_waits(): @pytest.mark.skipif( "platform.system() == 'Windows'", - reason="Expects signal -15 gets 15 at the last stop", + reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", ) def test_shell_started_server_stops(): """Test if executor terminates properly executor with shell=True.""" @@ -80,7 +80,7 @@ def test_shell_started_server_stops(): @pytest.mark.skipif( "platform.system() == 'Windows'", - reason="Expects signal -15 gets 15 at the last stop", + reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", ) @pytest.mark.parametrize("method", ("HEAD", "GET", "POST")) def test_slow_method_server_starting(method): @@ -106,7 +106,7 @@ def test_slow_method_server_starting(method): @pytest.mark.skipif( "platform.system() == 'Windows'", - reason="Expects signal -15 gets 15 at the last stop", + reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", ) def test_slow_post_payload_server_starting(): """ @@ -131,7 +131,8 @@ def test_slow_post_payload_server_starting(): @pytest.mark.skipif( - "platform.system() == 'Windows'", reason="No such process when stopping." + "platform.system() == 'Windows'", + reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", ) @pytest.mark.parametrize("method", ("HEAD", "GET", "POST")) def test_slow_method_server_timed_out(method): @@ -152,7 +153,8 @@ def test_slow_method_server_timed_out(method): @pytest.mark.skipif( - "platform.system() == 'Windows'", reason="No such process when stopping." + "platform.system() == 'Windows'", + reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", ) def test_fail_if_other_running(): """Test raising AlreadyRunning exception when port is blocked.""" @@ -179,7 +181,8 @@ def test_fail_if_other_running(): @pytest.mark.skipif( - "platform.system() == 'Windows'", reason="No such process when stopping." + "platform.system() == 'Windows'", + reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", ) @patch.object(HTTPExecutor, "DEFAULT_PORT", PORT) def test_default_port(): @@ -201,7 +204,8 @@ def test_default_port(): @pytest.mark.skipif( - "platform.system() == 'Windows'", reason="No such process when stopping." + "platform.system() == 'Windows'", + reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", ) @pytest.mark.parametrize( "accepted_status, expected_timeout", From c77843679128b12c52c6e22b51b3ea69d49490e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Thu, 13 May 2021 18:21:47 +0200 Subject: [PATCH 5/9] Translate psutil.NoSuchProcess to OsError(errno.ESRCH) --- src/mirakuru/compat.py | 10 ------ src/mirakuru/kill.py | 21 ++++++++---- tests/executors/test_executor.py | 16 --------- tests/executors/test_executor_kill.py | 4 +-- tests/executors/test_http_executor.py | 48 ++++++++++++--------------- 5 files changed, 37 insertions(+), 62 deletions(-) diff --git a/src/mirakuru/compat.py b/src/mirakuru/compat.py index 062060ab..1ad90e04 100644 --- a/src/mirakuru/compat.py +++ b/src/mirakuru/compat.py @@ -16,20 +16,10 @@ # You should have received a copy of the GNU Lesser General Public License # along with mirakuru. If not, see . """Mirakuru compatibility module.""" -import platform import signal -IS_WINDOWS = platform.system() == "Windows" - # Windows does not have SIGKILL, fall back to SIGTERM. SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM) -def returncode_from_signal(signal: int) -> int: - """Calculates expected exit code based on signal. Used as a default behaviour in""" - if IS_WINDOWS: - return signal - return -signal - - __all__ = ("SIGKILL",) diff --git a/src/mirakuru/kill.py b/src/mirakuru/kill.py index aaa0c3d7..cd7294fb 100644 --- a/src/mirakuru/kill.py +++ b/src/mirakuru/kill.py @@ -1,3 +1,4 @@ +import errno import os from typing import List @@ -12,10 +13,16 @@ if psutil: def killpg(pid: int, sig: int) -> None: - process = psutil.Process(pid) - children: List[psutil.Process] = process.children(recursive=True) - for child in children: - child.send_signal(sig) - psutil.wait_procs(children, timeout=30) - process.send_signal(sig) - process.wait(timeout=30) + """Custom killpg implementation for Windows.""" + try: + process = psutil.Process(pid) + children: List[psutil.Process] = process.children( + recursive=True + ) + for child in children: + child.send_signal(sig) + psutil.wait_procs(children, timeout=30) + process.send_signal(sig) + process.wait(timeout=30) + except psutil.NoSuchProcess as exc: + raise OSError(errno.ESRCH, exc.msg) from exc diff --git a/tests/executors/test_executor.py b/tests/executors/test_executor.py index f902b8af..d1026d91 100644 --- a/tests/executors/test_executor.py +++ b/tests/executors/test_executor.py @@ -151,14 +151,6 @@ def test_context_stopped(): ), pytest.param( shlex.split(ECHO_FOOBAR), - marks=pytest.mark.skipif( - "platform.system() == 'Windows'", - reason=( - "psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists. It's Echo, " - "so at the moment of psutil.Process creation " - "to kill it, it's already stopped." - ), - ), ), ), ) @@ -183,14 +175,6 @@ def test_process_output(command): ), pytest.param( shlex.split(ECHO_FOOBAR), - marks=pytest.mark.skipif( - "platform.system() == 'Windows'", - reason=( - "psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists. It's Echo, " - "so at the moment of psutil.Process creation " - "to kill it, it's already stopped." - ), - ), ), ), ) diff --git a/tests/executors/test_executor_kill.py b/tests/executors/test_executor_kill.py index 8abd4a3a..32730df9 100644 --- a/tests/executors/test_executor_kill.py +++ b/tests/executors/test_executor_kill.py @@ -49,8 +49,8 @@ def test_kill_custom_signal_kill(): @pytest.mark.skipif( "platform.system() == 'Windows'", reason=( - "psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists. It's Echo, so at the moment " - "of psutil.Process creation to kill it, it's already stopped." + "Failed: DID NOT RAISE " + "" ), ) def test_already_closed(): diff --git a/tests/executors/test_http_executor.py b/tests/executors/test_http_executor.py index 88794727..d46b57d8 100644 --- a/tests/executors/test_http_executor.py +++ b/tests/executors/test_http_executor.py @@ -55,10 +55,6 @@ def test_executor_starts_and_waits(): assert command in str(executor) -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", -) def test_shell_started_server_stops(): """Test if executor terminates properly executor with shell=True.""" executor = HTTPExecutor( @@ -78,10 +74,6 @@ def test_shell_started_server_stops(): connect_to_server() -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", -) @pytest.mark.parametrize("method", ("HEAD", "GET", "POST")) def test_slow_method_server_starting(method): """ @@ -104,10 +96,6 @@ def test_slow_method_server_starting(method): connect_to_server() -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", -) def test_slow_post_payload_server_starting(): """ Test whether or not executor awaits for slow starting servers. @@ -132,7 +120,7 @@ def test_slow_post_payload_server_starting(): @pytest.mark.skipif( "platform.system() == 'Windows'", - reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", + reason=("ProcessLookupError: [Errno 3] process no longer exists."), ) @pytest.mark.parametrize("method", ("HEAD", "GET", "POST")) def test_slow_method_server_timed_out(method): @@ -152,10 +140,6 @@ def test_slow_method_server_timed_out(method): assert "timed out after" in str(exc.value) -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", -) def test_fail_if_other_running(): """Test raising AlreadyRunning exception when port is blocked.""" executor = HTTPExecutor( @@ -180,10 +164,6 @@ def test_fail_if_other_running(): assert "seems to be already running" in str(exc.value) -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", -) @patch.object(HTTPExecutor, "DEFAULT_PORT", PORT) def test_default_port(): """ @@ -203,17 +183,31 @@ def test_default_port(): executor.stop() -@pytest.mark.skipif( - "platform.system() == 'Windows'", - reason="psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists.", -) @pytest.mark.parametrize( "accepted_status, expected_timeout", ( # default behaviour - only 2XX HTTP status codes are accepted - (None, True), + pytest.param( + None, + True, + marks=pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=( + "ProcessLookupError: [Errno 3] process no longer exists." + ), + ), + ), # one explicit integer status code - (200, True), + pytest.param( + 200, + True, + marks=pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=( + "ProcessLookupError: [Errno 3] process no longer exists." + ), + ), + ), # one explicit status code as a string ("404", False), # status codes as a regular expression From 11d1bf61983bdbfe14d82ac74df7a1e1f822188e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Fri, 14 May 2021 14:27:03 +0200 Subject: [PATCH 6/9] Make sure all existing children will get signal send --- src/mirakuru/base.py | 9 ++++++--- src/mirakuru/kill.py | 6 +++++- tests/executors/test_executor_kill.py | 2 -- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/mirakuru/base.py b/src/mirakuru/base.py index 8786404c..72a22179 100644 --- a/src/mirakuru/base.py +++ b/src/mirakuru/base.py @@ -145,6 +145,7 @@ def __init__( # pylint:disable=too-many-arguments is a good practice to set this. """ + self.__delete = False if isinstance(command, (list, tuple)): self.command = " ".join((shlex.quote(c) for c in command)) """Command that the executor runs.""" @@ -201,7 +202,6 @@ def running(self) -> bool: :rtype: bool """ if self.process is None: - LOG.debug("There is no process running!") return False return self.process.poll() is None @@ -300,7 +300,8 @@ def _kill_all_kids(self, sig: int) -> Set[int]: """ pids = processes_with_env(ENV_UUID, self._uuid) for pid in pids: - LOG.debug("Killing process %d ...", pid) + if not self.__delete: + LOG.debug("Killing process %d ...", pid) try: os.kill(pid, sig) except OSError as err: @@ -309,7 +310,8 @@ def _kill_all_kids(self, sig: int) -> Set[int]: pass else: raise - LOG.debug("Killed process %d.", pid) + if not self.__delete: + LOG.debug("Killed process %d.", pid) return pids def stop( @@ -473,6 +475,7 @@ def check_timeout(self) -> bool: def __del__(self) -> None: """Cleanup subprocesses created during Executor lifetime.""" + self.__delete = True try: if self.process: self.kill() diff --git a/src/mirakuru/kill.py b/src/mirakuru/kill.py index cd7294fb..ea93282f 100644 --- a/src/mirakuru/kill.py +++ b/src/mirakuru/kill.py @@ -20,7 +20,11 @@ def killpg(pid: int, sig: int) -> None: recursive=True ) for child in children: - child.send_signal(sig) + try: + child.send_signal(sig) + except psutil.NoSuchProcess: + # Already killed + pass psutil.wait_procs(children, timeout=30) process.send_signal(sig) process.wait(timeout=30) diff --git a/tests/executors/test_executor_kill.py b/tests/executors/test_executor_kill.py index 32730df9..c5859c6e 100644 --- a/tests/executors/test_executor_kill.py +++ b/tests/executors/test_executor_kill.py @@ -6,10 +6,8 @@ import errno -import os from unittest.mock import patch -import psutil import pytest from mirakuru import SimpleExecutor, HTTPExecutor From c9553ba59798a0da5df0996f8e26c82395dd6ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Fri, 14 May 2021 15:09:08 +0200 Subject: [PATCH 7/9] Remove caplog --- tests/executors/test_tcp_executor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/executors/test_tcp_executor.py b/tests/executors/test_tcp_executor.py index 3166e158..6e3c3c59 100644 --- a/tests/executors/test_tcp_executor.py +++ b/tests/executors/test_tcp_executor.py @@ -25,9 +25,8 @@ "platform.system() == 'Windows'", reason="select has no attribute poll", ) -def test_start_and_wait(caplog: LogCaptureFixture): +def test_start_and_wait(): """Test if executor await for process to accept connections.""" - caplog.set_level(logging.DEBUG, logger="mirakuru") executor = TCPExecutor(NC_COMMAND, "localhost", port=3000, timeout=5) executor.start() assert executor.running() is True From 8c039b72591e56dbbc60fcfc410c326436a6338c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Fri, 14 May 2021 15:23:46 +0200 Subject: [PATCH 8/9] catch and clear --- src/mirakuru/base.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mirakuru/base.py b/src/mirakuru/base.py index 72a22179..18818b17 100644 --- a/src/mirakuru/base.py +++ b/src/mirakuru/base.py @@ -478,7 +478,10 @@ def __del__(self) -> None: self.__delete = True try: if self.process: - self.kill() + try: + self.kill() + except OSError: + self._clear_process() except Exception: # pragma: no cover print("*" * 80) print( From f7e2ec9c780f100e2c3fb21c3c15d5841eb641da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Aliwi=C5=84ski?= Date: Fri, 14 May 2021 16:09:16 +0200 Subject: [PATCH 9/9] Unskip exit signal handling --- tests/executors/test_executor.py | 6 ------ tests/executors/test_executor_kill.py | 5 ++++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/executors/test_executor.py b/tests/executors/test_executor.py index d1026d91..8a83053f 100644 --- a/tests/executors/test_executor.py +++ b/tests/executors/test_executor.py @@ -19,9 +19,6 @@ SLEEP_300 = "sleep 300" -@pytest.mark.skipif( - "platform.system() == 'Windows'", reason="Expects signal -15 gets 15" -) @pytest.mark.parametrize("command", (SLEEP_300, SLEEP_300.split())) def test_running_process(command): """Start process and shuts it down.""" @@ -36,9 +33,6 @@ def test_running_process(command): assert SLEEP_300 in str(executor) -@pytest.mark.skipif( - "platform.system() == 'Windows'", reason="Expects signal -15 gets 15" -) @pytest.mark.parametrize("command", (SLEEP_300, SLEEP_300.split())) def test_command(command): """Check that the command and command parts are equivalent.""" diff --git a/tests/executors/test_executor_kill.py b/tests/executors/test_executor_kill.py index c5859c6e..4887f6b1 100644 --- a/tests/executors/test_executor_kill.py +++ b/tests/executors/test_executor_kill.py @@ -91,7 +91,10 @@ def test_daemons_killing(): @pytest.mark.skipif( "platform.system() == 'Windows'", - reason="Expects signal -15 gets 15 at the last stop", + reason=( + "Subprocess killed earlier than in 10 secs. " + "Blocking signals probably doesn't work." + ), ) def test_stopping_brutally(): """