diff --git a/.github/workflows/tests-windows.yml b/.github/workflows/tests-windows.yml new file mode 100644 index 0000000..ab9d825 --- /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 diff --git a/mirakuru/base.py b/mirakuru/base.py index 2dc586d..2dfe41e 100644 --- a/mirakuru/base.py +++ b/mirakuru/base.py @@ -45,13 +45,14 @@ ) from mirakuru.base_env import processes_with_env -from mirakuru.compat import SIGKILL from mirakuru.exceptions import ( AlreadyRunning, ProcessExitedWithError, ProcessFinishedWithError, TimeoutExpired, ) +from mirakuru.compat import SIGKILL +from mirakuru.kill import killpg LOG = logging.getLogger(__name__) @@ -64,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") @@ -141,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.""" @@ -195,7 +200,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 @@ -290,7 +294,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: @@ -299,7 +304,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( @@ -330,7 +336,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 @@ -359,10 +365,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) @@ -398,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() @@ -452,9 +462,13 @@ def check_timeout(self) -> bool: def __del__(self) -> None: """Cleanup subprocesses created during Executor lifetime.""" + 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("Exception while deleting Executor. It is strongly suggested that you use") diff --git a/mirakuru/kill.py b/mirakuru/kill.py new file mode 100644 index 0000000..ea93282 --- /dev/null +++ b/mirakuru/kill.py @@ -0,0 +1,32 @@ +import errno +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: int, sig: int) -> None: + """Custom killpg implementation for Windows.""" + try: + process = psutil.Process(pid) + children: List[psutil.Process] = process.children( + recursive=True + ) + for child in children: + 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) + 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 4851af16..0a7ff89 100644 --- a/tests/executors/test_executor.py +++ b/tests/executors/test_executor.py @@ -41,6 +41,9 @@ def test_command(command: Union[str, List[str]]) -> None: assert executor.command_parts == SLEEP_300.split() +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_custom_signal_stop() -> None: """Start process and shuts it down using signal SIGQUIT.""" executor = SimpleExecutor(SLEEP_300, stop_signal=signal.SIGQUIT) @@ -50,6 +53,9 @@ def test_custom_signal_stop() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_stop_custom_signal_stop() -> None: """Start process and shuts it down using signal SIGQUIT passed to stop.""" executor = SimpleExecutor(SLEEP_300) @@ -59,6 +65,9 @@ def test_stop_custom_signal_stop() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_stop_custom_exit_signal_stop() -> None: """Start process and expect it to finish with custom signal.""" executor = SimpleExecutor("false", shell=True) @@ -68,6 +77,9 @@ def test_stop_custom_exit_signal_stop() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_stop_custom_exit_signal_context() -> None: """Start process and expect custom exit signal in context manager.""" with SimpleExecutor("false", expected_returncode=-3, shell=True) as executor: @@ -75,6 +87,9 @@ def test_stop_custom_exit_signal_context() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_running_context() -> None: """Start process and shuts it down.""" executor = SimpleExecutor(SLEEP_300) @@ -84,12 +99,18 @@ def test_running_context() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_executor_in_context_only() -> None: """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() -> None: """Start for context, and shuts it for nested context.""" executor = SimpleExecutor(SLEEP_300) @@ -105,7 +126,21 @@ def test_context_stopped() -> None: 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), + ), + ), +) def test_process_output(command: Union[str, List[str]]) -> None: """Start process, check output and shut it down.""" executor = SimpleExecutor(command) @@ -115,7 +150,21 @@ def test_process_output(command: Union[str, List[str]]) -> None: 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), + ), + ), +) def test_process_output_shell(command: Union[str, List[str]]) -> None: """Start process, check output and shut it down with shell set to True.""" executor = SimpleExecutor(command, shell=True) @@ -147,6 +196,7 @@ def test_stopping_not_yet_running_executor() -> None: executor.stop() +@pytest.mark.skipif("platform.system() == 'Windows'", reason="No ps_uax") def test_forgotten_stop() -> None: """Test if SimpleExecutor subprocess is killed after an instance is deleted. @@ -235,6 +285,7 @@ def test_executor_methods_returning_self() -> None: assert SimpleExecutor(SLEEP_300).start().stop().output +@pytest.mark.skipif("platform.system() == 'Windows'", reason="No ps_uax") def test_mirakuru_cleanup() -> None: """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 f9e3287..e9ac3e2 100644 --- a/tests/executors/test_executor_kill.py +++ b/tests/executors/test_executor_kill.py @@ -13,11 +13,16 @@ from mirakuru import HTTPExecutor, SimpleExecutor from mirakuru.compat import SIGKILL from mirakuru.exceptions import ProcessFinishedWithError +from mirakuru.kill import killpg + from tests import SAMPLE_DAEMON_PATH, TEST_SERVER_PATH, ps_aux SLEEP_300 = "sleep 300" +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_custom_signal_kill() -> None: """Start process and shuts it down using signal SIGQUIT.""" executor = SimpleExecutor(SLEEP_300, kill_signal=signal.SIGQUIT) @@ -27,6 +32,9 @@ def test_custom_signal_kill() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", reason="No SIGQUIT support for Windows" +) def test_kill_custom_signal_kill() -> None: """Start process and shuts it down using signal SIGQUIT passed to kill.""" executor = SimpleExecutor(SLEEP_300) @@ -36,12 +44,19 @@ def test_kill_custom_signal_kill() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=( + "Failed: DID NOT RAISE " + "" + ), +) def test_already_closed() -> None: """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() -> bool: """Return True only only when self.process is not running.""" @@ -53,6 +68,7 @@ def process_stopped() -> bool: assert not executor.process +@pytest.mark.skipif("platform.system() == 'Windows'", reason="No ps_uax") def test_daemons_killing() -> None: """Test if all subprocesses of SimpleExecutor can be killed. @@ -72,6 +88,13 @@ def test_daemons_killing() -> None: assert SAMPLE_DAEMON_PATH not in ps_aux() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=( + "Subprocess killed earlier than in 10 secs. " + "Blocking signals probably doesn't work." + ), +) def test_stopping_brutally() -> None: """Test if SimpleExecutor is stopping insubordinate process. diff --git a/tests/executors/test_http_executor.py b/tests/executors/test_http_executor.py index d8eacd8..e1176c4 100644 --- a/tests/executors/test_http_executor.py +++ b/tests/executors/test_http_executor.py @@ -34,6 +34,10 @@ def connect_to_server() -> None: conn.close() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Can't start http.server on python3", +) def test_executor_starts_and_waits() -> None: """Test if process awaits for HEAD request to be completed.""" command = f'bash -c "sleep 3 && {HTTP_NORMAL_CMD}"' @@ -104,6 +108,10 @@ def test_slow_post_payload_server_starting() -> None: connect_to_server() +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason=("ProcessLookupError: [Errno 3] process no longer exists."), +) @pytest.mark.parametrize("method", ("HEAD", "GET", "POST")) def test_slow_method_server_timed_out(method: str) -> None: """Check if timeout properly expires.""" @@ -164,9 +172,27 @@ def test_default_port() -> None: "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 diff --git a/tests/executors/test_output_executor.py b/tests/executors/test_output_executor.py index 6b3c643..1be8210 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() -> None: """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() -> None: 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() -> None: """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() -> None: assert "foo" in str(executor) +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_executor_dont_start() -> None: """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 fb822db..3d5839b 100644 --- a/tests/executors/test_pid_executor.py +++ b/tests/executors/test_pid_executor.py @@ -31,6 +31,10 @@ def run_around_tests() -> Iterator[None]: pass +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_start_and_wait() -> None: """Test if the executor will await for the process to create a file.""" process = f'bash -c "sleep 2 && touch {FILENAME} && sleep 10"' @@ -49,6 +53,10 @@ def test_empty_filename(pid_file: Optional[str]) -> None: PidExecutor(SLEEP, pid_file) # type: ignore[arg-type] +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_if_file_created() -> None: """Check whether the process really created the given file.""" assert os.path.isfile(FILENAME) is False @@ -57,6 +65,10 @@ def test_if_file_created() -> None: assert os.path.isfile(FILENAME) is True +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_timeout_error() -> None: """Check if timeout properly expires.""" executor = PidExecutor(SLEEP, FILENAME, timeout=1) @@ -67,6 +79,10 @@ def test_timeout_error() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_fail_if_other_executor_running() -> None: """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 2daf73a..4a9b553 100644 --- a/tests/executors/test_tcp_executor.py +++ b/tests/executors/test_tcp_executor.py @@ -18,9 +18,12 @@ 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) -> None: """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 @@ -35,6 +38,10 @@ def test_repr_and_str() -> None: 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() -> None: """Check if TimeoutExpired gets raised correctly.""" command = 'bash -c "sleep 10 && nc -lk 3000"' @@ -46,6 +53,10 @@ def test_it_raises_error_on_timeout() -> None: assert executor.running() is False +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="select has no attribute poll", +) def test_fail_if_other_executor_running() -> None: """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 6328b9a..a7c41cf 100644 --- a/tests/executors/test_unixsocket_executor.py +++ b/tests/executors/test_unixsocket_executor.py @@ -17,6 +17,10 @@ SOCKET_SERVER_CMD = f"{sys.executable} {TEST_SOCKET_SERVER_PATH} {SOCKET_PATH}" +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Python does not support socket.AF_UNIX on windows", +) def test_start_and_wait() -> None: """Test if executor await for process to accept connections.""" executor = UnixSocketExecutor(SOCKET_SERVER_CMD + " 2", socket_name=SOCKET_PATH, timeout=5) @@ -24,6 +28,10 @@ def test_start_and_wait() -> None: assert executor.running() is True +@pytest.mark.skipif( + "platform.system() == 'Windows'", + reason="Python does not support socket.AF_UNIX on windows", +) def test_start_and_timeout() -> None: """Test if executor will properly times out.""" executor = UnixSocketExecutor(SOCKET_SERVER_CMD + " 10", socket_name=SOCKET_PATH, timeout=5)