Refactor PostgreSQLExecutor to support Windows compatibility for process management

- Introduced _get_base_command method to dynamically set the base command for PostgreSQL based on the operating system.
- Added _windows_terminate_process method to handle process termination on Windows, ensuring graceful shutdown before force-killing.
- Updated stop method to utilize the new Windows-specific termination logic.

Author: tboy1337

pytest_postgresql/executor.py

@@ -24,6 +24,8 @@
 import subprocess
 import tempfile
 import time
+import os
+import signal
 from typing import Any, Optional, TypeVar
 
 from mirakuru import TCPExecutor
@@ -48,13 +50,28 @@ class PostgreSQLExecutor(TCPExecutor):
     <http://www.postgresql.org/docs/current/static/app-pg-ctl.html>`_
     """
 
-    BASE_PROC_START_COMMAND = (
-        '{executable} start -D "{datadir}" '
-        "-o \"-F -p {port} -c log_destination='stderr' "
-        "-c logging_collector=off "
-        "-c unix_socket_directories='{unixsocketdir}' {postgres_options}\" "
-        '-l "{logfile}" {startparams}'
-    )
+    def _get_base_command(self) -> str:
+        """Get the base PostgreSQL command, Windows-compatible."""
+        if platform.system() == "Windows":
+            # Windows doesn't handle single quotes well in subprocess calls
+            return (
+                '{executable} start -D "{datadir}" '
+                "-o \"-F -p {port} -c log_destination=stderr "
+                "-c logging_collector=off "
+                "-c unix_socket_directories={unixsocketdir} {postgres_options}\" "
+                '-l "{logfile}" {startparams}'
+            )
+        else:
+            # Unix/Linux systems work with single quotes
+            return (
+                '{executable} start -D "{datadir}" '
+                "-o \"-F -p {port} -c log_destination='stderr' "
+                "-c logging_collector=off "
+                "-c unix_socket_directories='{unixsocketdir}' {postgres_options}\" "
+                '-l "{logfile}" {startparams}'
+            )
+
+    BASE_PROC_START_COMMAND = ""  # Will be set dynamically
 
     VERSION_RE = re.compile(r".* (?P<version>\d+(?:\.\d+)?)")
     MIN_SUPPORTED_VERSION = parse("10")
@@ -108,7 +125,7 @@ def __init__(
         self.logfile = logfile
         self.startparams = startparams
         self.postgres_options = postgres_options
-        command = self.BASE_PROC_START_COMMAND.format(
+        command = self._get_base_command().format(
             executable=self.executable,
             datadir=self.datadir,
             port=port,
@@ -219,17 +236,50 @@ def running(self) -> bool:
         status_code = subprocess.getstatusoutput(f'{self.executable} status -D "{self.datadir}"')[0]
         return status_code == 0
 
+    def _windows_terminate_process(self, sig: Optional[int] = None) -> None:
+        """Terminate process on Windows."""
+        if self.process is None:
+            return
+        
+        try:
+            if platform.system() == "Windows":
+                # On Windows, try to terminate gracefully first
+                self.process.terminate()
+                # Give it a chance to terminate gracefully
+                try:
+                    self.process.wait(timeout=5)
+                except subprocess.TimeoutExpired:
+                    # If it doesn't terminate gracefully, force kill
+                    self.process.kill()
+                    self.process.wait()
+            else:
+                # On Unix systems, use the signal
+                actual_sig = sig or signal.SIGTERM
+                os.killpg(self.process.pid, actual_sig)
+        except (OSError, AttributeError):
+            # Process might already be dead or other issues
+            pass
+
     def stop(self: T, sig: Optional[int] = None, exp_sig: Optional[int] = None) -> T:
         """Issue a stop request to executable."""
         subprocess.check_output(
             f'{self.executable} stop -D "{self.datadir}" -m f',
             shell=True,
         )
         try:
-            super().stop(sig, exp_sig)
+            if platform.system() == "Windows":
+                self._windows_terminate_process(sig)
+            else:
+                super().stop(sig, exp_sig)
         except ProcessFinishedWithError:
             # Finished, leftovers ought to be cleaned afterwards anyway
             pass
+        except AttributeError as e:
+            # Handle case where os.killpg doesn't exist (shouldn't happen now)
+            if "killpg" in str(e):
+                self._windows_terminate_process(sig)
+            else:
+                raise
         return self
 
     def __del__(self) -> None:

tests/test_windows_compatibility.py

@@ -0,0 +1,213 @@
+"""Test Windows compatibility fixes for pytest-postgresql."""
+
+import platform
+import pytest
+from unittest.mock import Mock, patch, MagicMock
+import subprocess
+import signal
+
+from pytest_postgresql.executor import PostgreSQLExecutor
+
+
+class TestWindowsCompatibility:
+    """Test Windows-specific functionality."""
+
+    def test_get_base_command_windows(self):
+        """Test that Windows base command doesn't use single quotes."""
+        executor = PostgreSQLExecutor(
+            executable="/path/to/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+        
+        with patch('platform.system', return_value='Windows'):
+            command = executor._get_base_command()
+            # Windows command should not have single quotes around stderr
+            assert "log_destination=stderr" in command
+            assert "log_destination='stderr'" not in command
+            assert "unix_socket_directories={unixsocketdir}" in command
+            assert "unix_socket_directories='{unixsocketdir}'" not in command
+
+    def test_get_base_command_unix(self):
+        """Test that Unix base command uses single quotes."""
+        executor = PostgreSQLExecutor(
+            executable="/path/to/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+        
+        with patch('platform.system', return_value='Linux'):
+            command = executor._get_base_command()
+            # Unix command should have single quotes around stderr
+            assert "log_destination='stderr'" in command
+            assert "unix_socket_directories='{unixsocketdir}'" in command
+
+    def test_windows_terminate_process(self):
+        """Test Windows process termination."""
+        executor = PostgreSQLExecutor(
+            executable="/path/to/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+        
+        # Mock process
+        mock_process = MagicMock()
+        executor.process = mock_process
+        
+        with patch('platform.system', return_value='Windows'):
+            executor._windows_terminate_process()
+            
+            # Should call terminate first
+            mock_process.terminate.assert_called_once()
+            mock_process.wait.assert_called()
+
+    def test_windows_terminate_process_force_kill(self):
+        """Test Windows process termination with force kill on timeout."""
+        executor = PostgreSQLExecutor(
+            executable="/path/to/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+        
+        # Mock process that times out
+        mock_process = MagicMock()
+        mock_process.wait.side_effect = [subprocess.TimeoutExpired(cmd="test", timeout=5), None]
+        executor.process = mock_process
+        
+        with patch('platform.system', return_value='Windows'):
+            executor._windows_terminate_process()
+            
+            # Should call terminate, wait (timeout), then kill, then wait again
+            mock_process.terminate.assert_called_once()
+            mock_process.kill.assert_called_once()
+            assert mock_process.wait.call_count == 2
+
+    def test_stop_method_windows(self):
+        """Test stop method on Windows."""
+        executor = PostgreSQLExecutor(
+            executable="/path/to/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+        
+        # Mock subprocess and process
+        with patch('subprocess.check_output') as mock_subprocess, \
+             patch('platform.system', return_value='Windows'), \
+             patch.object(executor, '_windows_terminate_process') as mock_terminate:
+            
+            result = executor.stop()
+            
+            # Should call pg_ctl stop and Windows terminate
+            mock_subprocess.assert_called_once()
+            mock_terminate.assert_called_once_with(None)
+            assert result is executor
+
+    def test_stop_method_unix(self):
+        """Test stop method on Unix systems."""
+        executor = PostgreSQLExecutor(
+            executable="/path/to/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+        
+        # Mock subprocess and super().stop
+        with patch('subprocess.check_output') as mock_subprocess, \
+             patch('platform.system', return_value='Linux'), \
+             patch('pytest_postgresql.executor.TCPExecutor.stop') as mock_super_stop:
+            
+            mock_super_stop.return_value = executor
+            result = executor.stop()
+            
+            # Should call pg_ctl stop and parent class stop
+            mock_subprocess.assert_called_once()
+            mock_super_stop.assert_called_once_with(None, None)
+            assert result is executor
+
+    def test_stop_method_fallback_on_killpg_error(self):
+        """Test stop method falls back to Windows termination on killpg AttributeError."""
+        executor = PostgreSQLExecutor(
+            executable="/path/to/pg_ctl",
+            host="localhost",
+            port=5432,
+            datadir="/tmp/data",
+            unixsocketdir="/tmp/socket",
+            logfile="/tmp/log",
+            startparams="-w",
+            dbname="test",
+        )
+        
+        # Mock subprocess and super().stop to raise AttributeError
+        with patch('subprocess.check_output') as mock_subprocess, \
+             patch('platform.system', return_value='Linux'), \
+             patch('pytest_postgresql.executor.TCPExecutor.stop', 
+                   side_effect=AttributeError("module 'os' has no attribute 'killpg'")), \
+             patch.object(executor, '_windows_terminate_process') as mock_terminate:
+            
+            result = executor.stop()
+            
+            # Should call pg_ctl stop, fail on super().stop, then use Windows terminate
+            mock_subprocess.assert_called_once()
+            mock_terminate.assert_called_once_with(None)
+            assert result is executor
+
+    def test_command_formatting_windows(self):
+        """Test that command is properly formatted for Windows."""
+        with patch('platform.system', return_value='Windows'):
+            executor = PostgreSQLExecutor(
+                executable="C:/Program Files/PostgreSQL/bin/pg_ctl.exe",
+                host="localhost",
+                port=5555,
+                datadir="C:/temp/data",
+                unixsocketdir="C:/temp/socket",
+                logfile="C:/temp/log.txt",
+                startparams="-w -s",
+                dbname="testdb",
+                postgres_options="-c shared_preload_libraries=test"
+            )
+            
+            # The command should be properly formatted without quotes around stderr
+            expected_parts = [
+                "C:/Program Files/PostgreSQL/bin/pg_ctl.exe start",
+                '-D "C:/temp/data"',
+                "-o \"-F -p 5555 -c log_destination=stderr",
+                "-c logging_collector=off",
+                "-c unix_socket_directories=C:/temp/socket",
+                "-c shared_preload_libraries=test\"",
+                '-l "C:/temp/log.txt"',
+                "-w -s"
+            ]
+            
+            # Check if all expected parts are in the command
+            command = executor.command
+            for part in expected_parts:
+                assert part in command, f"Expected '{part}' in command: {command}"