Windows support

.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

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")

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

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,13 +77,19 @@ 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:
         executor.stop(stop_signal=signal.SIGQUIT)
         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"""

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 "
+        "<class 'mirakuru.exceptions.ProcessFinishedWithError'>"
+    ),
+)
 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.