Refactor PostgreSQLExecutor to support Windows compatibility for process management

.github/workflows/single-postgres.yml

@@ -43,24 +43,42 @@ jobs:
       - uses: ankane/setup-postgres@v1
         with:
           postgres-version: ${{ inputs.postgresql }}
+      - name: Detect PostgreSQL path on Windows
+        if: runner.os == 'Windows'
+        shell: pwsh
+        run: |
+          $pgPath = "C:\Program Files\PostgreSQL\${{ inputs.postgresql }}\bin\pg_ctl"
+          if (Test-Path $pgPath) {
+            echo "POSTGRESQL_EXEC=$pgPath" >> $env:GITHUB_ENV
+          } else {
+            $pgPath = (Get-Command pg_ctl -ErrorAction SilentlyContinue).Source
+            if ($pgPath) {
+              echo "POSTGRESQL_EXEC=$pgPath" >> $env:GITHUB_ENV
+            }
+          }
+      - name: Set PostgreSQL path for Unix/macOS
+        if: runner.os != 'Windows'
+        run: |
+          echo "POSTGRESQL_EXEC=/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" >> $GITHUB_ENV
       - name: Check installed locales
+        if: runner.os != 'Windows'
         run: |
           locale -a
       - name: update locale for tests
         if: ${{ inputs.os == 'ubuntu-latest' }}
         run: |
           sudo locale-gen de_DE.UTF-8
       - name: install libpq
-        if: ${{ contains(inputs.python-versions, 'pypy') }}
+        if: ${{ contains(inputs.python-versions, 'pypy') && runner.os == 'Linux' }}
         run: sudo apt install libpq5
       - name: Run test
         uses: fizyk/actions-reuse/.github/actions/pipenv-run@v4.1.1
         with:
-          command: pytest -svv -p no:xdist --postgresql-exec="/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" -k "not docker" --cov-report=xml
+          command: pytest -svv -p no:xdist --postgresql-exec="${{ env.POSTGRESQL_EXEC }}" -k "not docker" --cov-report=xml
       - name: Run xdist test
         uses: fizyk/actions-reuse/.github/actions/pipenv-run@v4.1.1
         with:
-          command: pytest -n auto --dist loadgroup --max-worker-restart 0 --postgresql-exec="/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" -k "not docker" --cov-report=xml:coverage-xdist.xml
+          command: pytest -n auto --dist loadgroup --max-worker-restart 0 --postgresql-exec="${{ env.POSTGRESQL_EXEC }}" -k "not docker" --cov-report=xml:coverage-xdist.xml
       - uses: actions/upload-artifact@v6
         if: failure()
         with:

.github/workflows/tests.yml

@@ -59,6 +59,27 @@ jobs:
       postgresql: 16
       os: macos-latest
       python-versions: '["3.13", "3.14"]'
+  windows_postgres_18:
+    needs: [postgresql_18]
+    uses: ./.github/workflows/single-postgres.yml
+    with:
+      postgresql: 18
+      os: windows-latest
+      python-versions: '["3.12", "3.13", "3.14"]'
+  windows_postgres_17:
+    needs: [postgresql_17, windows_postgres_18]
+    uses: ./.github/workflows/single-postgres.yml
+    with:
+      postgresql: 17
+      os: windows-latest
+      python-versions: '["3.12", "3.13", "3.14"]'
+  windows_postgres_16:
+    needs: [postgresql_16, windows_postgres_17]
+    uses: ./.github/workflows/single-postgres.yml
+    with:
+      postgresql: 16
+      os: windows-latest
+      python-versions: '["3.13", "3.14"]'
   docker_postgresql_18:
     needs: [postgresql_18]
     uses: ./.github/workflows/dockerised-postgres.yml

pytest_postgresql/executor.py

@@ -17,6 +17,7 @@
 # along with pytest-postgresql.  If not, see <http://www.gnu.org/licenses/>.
 """PostgreSQL executor crafter around pg_ctl."""
 
+import os
 import os.path
 import platform
 import re
@@ -48,11 +49,14 @@ class PostgreSQLExecutor(TCPExecutor):
     <http://www.postgresql.org/docs/current/static/app-pg-ctl.html>`_
     """
 
+    # Base PostgreSQL start command template - cross-platform compatible
+    # Use unified format without single quotes around values
+    # This format works on both Windows and Unix systems
     BASE_PROC_START_COMMAND = (
         '{executable} start -D "{datadir}" '
-        "-o \"-F -p {port} -c log_destination='stderr' "
+        '-o "-F -p {port} -c log_destination=stderr '
         "-c logging_collector=off "
-        "-c unix_socket_directories='{unixsocketdir}' {postgres_options}\" "
+        '-c unix_socket_directories={unixsocketdir} {postgres_options}" '
         '-l "{logfile}" {startparams}'
     )
 
@@ -219,17 +223,47 @@ 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.
+
+        :param _sig: Signal parameter (unused on Windows but included for consistency)
+        """
+        if self.process is None:
+            return
+
+        try:
+            # 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()
+        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,
+            [self.executable, "stop", "-D", self.datadir, "-m", "f"],
         )
         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:
+            # Fallback for edge cases where os.killpg doesn't exist
+            if "killpg" in str(e):
+                self._windows_terminate_process(sig)
+            else:
+                raise
         return self
 
     def __del__(self) -> None:

tests/conftest.py

@@ -4,7 +4,9 @@
 from pathlib import Path
 
 from pytest_postgresql import factories
-from pytest_postgresql.plugin import *  # noqa: F403,F401
+
+# Plugin is registered via entry point in pyproject.toml - no need to import here
+# from pytest_postgresql.plugin import *  # noqa: F403,F401
 
 pytest_plugins = ["pytester"]
 POSTGRESQL_VERSION = os.environ.get("POSTGRES", "13")

tests/test_windows_compatibility.py

@@ -0,0 +1,186 @@
+"""Test Windows compatibility fixes for pytest-postgresql."""
+
+import subprocess
+from unittest.mock import MagicMock, patch
+
+from pytest_postgresql.executor import PostgreSQLExecutor
+
+
+class TestWindowsCompatibility:
+    """Test Windows-specific functionality."""
+
+    def test_base_command_unified(self) -> None:
+        """Test that base command template is unified and cross-platform compatible."""
+        # The BASE_PROC_START_COMMAND should use the simplified format without single quotes
+        # around configuration values, which works on both Windows and Unix systems
+        command_template = PostgreSQLExecutor.BASE_PROC_START_COMMAND
+
+        # Should use simplified format without single quotes
+        assert "log_destination=stderr" in command_template
+        assert "log_destination='stderr'" not in command_template
+        assert "unix_socket_directories={unixsocketdir}" in command_template
+        assert "unix_socket_directories='{unixsocketdir}'" not in command_template
+
+    def test_windows_terminate_process(self) -> None:
+        """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
+
+        # No need to mock platform.system() since the method doesn't check it anymore
+        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) -> None:
+        """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
+
+        # No need to mock platform.system() since the method doesn't check it anymore
+        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) -> None:
+        """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()
+            assert result is executor
+
+    def test_stop_method_unix(self) -> None:
+        """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) -> None:
+        """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()
+            assert result is executor
+
+    def test_command_formatting_windows(self) -> None:
+        """Test that command is properly formatted for Windows paths."""
+        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 single quotes around values
+        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}"