From 548247937c9b10bd89d47ef2edf619340b56db0f Mon Sep 17 00:00:00 2001 From: tboy1337 Date: Tue, 1 Sep 2026 19:21:30 +0700 Subject: [PATCH 1/4] Discover pg_ctl on Windows without --postgresql-exec - closes #1399 The Linux default path never exists on Windows, and pg_config is often not on PATH, so the fixture could not start a server without an explicit executable. --- .github/actions/detect-pg-ctl/action.yml | 30 ---- .github/workflows/async-postgres.yml | 21 +-- .github/workflows/single-postgres-windows.yml | 7 +- .github/workflows/single-postgres.yml | 8 +- README.rst | 19 +- newsfragments/1399.feature.rst | 4 + pytest_postgresql/exceptions.py | 12 ++ pytest_postgresql/factories/_pg.py | 168 ++++++++++++++++-- pytest_postgresql/plugin.py | 3 +- tests/test_pg_exe.py | 103 ++++++++++- tests/test_windows_compatibility.py | 2 +- 11 files changed, 300 insertions(+), 77 deletions(-) delete mode 100644 .github/actions/detect-pg-ctl/action.yml create mode 100644 newsfragments/1399.feature.rst diff --git a/.github/actions/detect-pg-ctl/action.yml b/.github/actions/detect-pg-ctl/action.yml deleted file mode 100644 index d68bcf9a..00000000 --- a/.github/actions/detect-pg-ctl/action.yml +++ /dev/null @@ -1,30 +0,0 @@ -name: Detect PostgreSQL path on Windows -description: Set POSTGRESQL_EXEC for pg_ctl on Windows runners -inputs: - postgresql-version: - description: PostgreSQL major version - required: true -runs: - using: composite - steps: - - shell: pwsh - env: - POSTGRESQL_VERSION: ${{ inputs.postgresql-version }} - run: | # zizmor: ignore[github-env] - $pgFound = $false - $hardcodedPath = "C:\Program Files\PostgreSQL\$env:POSTGRESQL_VERSION\bin\pg_ctl.exe" - if (Test-Path $hardcodedPath) { - echo "POSTGRESQL_EXEC=$hardcodedPath" >> $env:GITHUB_ENV - $pgFound = $true - } else { - $pgPath = (Get-Command pg_ctl -ErrorAction SilentlyContinue).Source - if ($pgPath) { - echo "POSTGRESQL_EXEC=$pgPath" >> $env:GITHUB_ENV - $pgFound = $true - } - } - - if (-not $pgFound) { - Write-Error "Error: pg_ctl not found in expected locations. Checked hardcoded path and system PATH." - exit 1 - } diff --git a/.github/workflows/async-postgres.yml b/.github/workflows/async-postgres.yml index dd04a6d5..29b02cd8 100644 --- a/.github/workflows/async-postgres.yml +++ b/.github/workflows/async-postgres.yml @@ -48,30 +48,31 @@ jobs: run: | if command -v pg_ctl >/dev/null 2>&1; then PG_CTL_PATH=$(command -v pg_ctl) - echo "POSTGRESQL_EXEC=$PG_CTL_PATH" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=$PG_CTL_PATH" >> "$GITHUB_ENV" elif [ -f "/opt/homebrew/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" ]; then - echo "POSTGRESQL_EXEC=/opt/homebrew/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=/opt/homebrew/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> "$GITHUB_ENV" elif [ -f "/usr/local/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" ]; then - echo "POSTGRESQL_EXEC=/usr/local/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=/usr/local/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> "$GITHUB_ENV" elif [ -f "/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" ]; then - echo "POSTGRESQL_EXEC=/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" >> "$GITHUB_ENV" else echo "Error: pg_ctl not found in expected locations" exit 1 fi - - name: Detect PostgreSQL path on Windows - if: runner.os == 'Windows' - uses: ./.github/actions/detect-pg-ctl - with: - postgresql-version: ${{ inputs.postgresql }} - name: update locale for tests if: startsWith(inputs.os, 'ubuntu') run: | sudo locale-gen de_DE.UTF-8 - - name: Run async tests + - name: Run async tests on Unix + if: runner.os != 'Windows' uses: fizyk/actions-reuse/.github/actions/uv-run@7eefbd6ffc6c6787ad3be6ce93dcf475e3ae99b5 # v5.6.0 with: command: python -m pytest -svv -m asyncio -p no:xdist --postgresql-exec="${{ env.POSTGRESQL_EXEC }}" -k "not docker" --basetemp="${{ runner.temp }}/pytest-basetemp" + - name: Run async tests on Windows + if: runner.os == 'Windows' + uses: fizyk/actions-reuse/.github/actions/uv-run@7eefbd6ffc6c6787ad3be6ce93dcf475e3ae99b5 # v5.6.0 + with: + command: python -m pytest -svv -m asyncio -p no:xdist -k "not docker" --basetemp="${{ runner.temp }}/pytest-basetemp" - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: failure() with: diff --git a/.github/workflows/single-postgres-windows.yml b/.github/workflows/single-postgres-windows.yml index 0078203e..a75a7234 100644 --- a/.github/workflows/single-postgres-windows.yml +++ b/.github/workflows/single-postgres-windows.yml @@ -43,17 +43,14 @@ jobs: - uses: ankane/setup-postgres@v1 # zizmor: ignore[unpinned-uses] with: postgres-version: ${{ inputs.postgresql }} - - uses: ./.github/actions/detect-pg-ctl - with: - postgresql-version: ${{ inputs.postgresql }} - uses: fizyk/actions-reuse/.github/actions/uv-pytest-coverage@7eefbd6ffc6c6787ad3be6ce93dcf475e3ae99b5 # v5.6.0 with: - pytest-opts: -svv -p no:xdist --postgresql-exec="${{ env.POSTGRESQL_EXEC }}" -k "not docker" --basetemp="${{ runner.temp }}/pytest-basetemp" + pytest-opts: -svv -p no:xdist -k "not docker" --basetemp="${{ runner.temp }}/pytest-basetemp" data-file: .coverage.serial output-file: coverage-serial.xml - uses: fizyk/actions-reuse/.github/actions/uv-pytest-coverage@7eefbd6ffc6c6787ad3be6ce93dcf475e3ae99b5 # v5.6.0 with: - pytest-opts: -n auto --dist loadgroup --max-worker-restart 0 --postgresql-exec="${{ env.POSTGRESQL_EXEC }}" -k "not docker" --basetemp="${{ runner.temp }}/pytest-basetemp" + pytest-opts: -n auto --dist loadgroup --max-worker-restart 0 -k "not docker" --basetemp="${{ runner.temp }}/pytest-basetemp" coverage-config: .coveragerc data-file: .coverage.xdist output-file: coverage-xdist.xml diff --git a/.github/workflows/single-postgres.yml b/.github/workflows/single-postgres.yml index 6fbc9c36..981b030f 100644 --- a/.github/workflows/single-postgres.yml +++ b/.github/workflows/single-postgres.yml @@ -53,16 +53,16 @@ jobs: # Try to find pg_ctl dynamically for cross-platform compatibility if command -v pg_ctl >/dev/null 2>&1; then PG_CTL_PATH=$(command -v pg_ctl) - echo "POSTGRESQL_EXEC=$PG_CTL_PATH" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=$PG_CTL_PATH" >> "$GITHUB_ENV" elif [ -f "/opt/homebrew/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" ]; then # macOS Apple Silicon Homebrew path - echo "POSTGRESQL_EXEC=/opt/homebrew/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=/opt/homebrew/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> "$GITHUB_ENV" elif [ -f "/usr/local/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" ]; then # macOS Intel Homebrew path - echo "POSTGRESQL_EXEC=/usr/local/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=/usr/local/opt/postgresql@${{ inputs.postgresql }}/bin/pg_ctl" >> "$GITHUB_ENV" elif [ -f "/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" ]; then # Debian/Ubuntu path (fallback) - echo "POSTGRESQL_EXEC=/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" >> $GITHUB_ENV + echo "POSTGRESQL_EXEC=/usr/lib/postgresql/${{ inputs.postgresql }}/bin/pg_ctl" >> "$GITHUB_ENV" else echo "Error: pg_ctl not found in expected locations" exit 1 diff --git a/README.rst b/README.rst index 3a8777cc..3e80cd8d 100644 --- a/README.rst +++ b/README.rst @@ -343,7 +343,7 @@ You can define settings via fixture factory arguments, command line options, or - n/a - --postgresql-exec - postgresql_exec - - ``pg_config --bindir`` + ``pg_ctl`` + - discovered (see note) * - host - host - host @@ -440,13 +440,18 @@ You can define settings via fixture factory arguments, command line options, or .. note:: - If the ``executable`` factory argument is not provided, the plugin looks for ``pg_ctl`` first at - the configured path (``--postgresql-exec`` / the ``postgresql_exec`` ini option, - ``/usr/lib/postgresql/14/bin/pg_ctl`` by default), then in the directory reported by - ``pg_config --bindir``. + If the ``executable`` factory argument is not provided, the plugin looks for ``pg_ctl`` in + this order: - Only the ``executable`` factory argument is taken at face value; the other two are - verified to exist before being used. If neither is there, an ``ExecutableMissingException`` + 1. the configured path (``--postgresql-exec`` / the ``postgresql_exec`` ini option; + on Unix this defaults to ``/usr/lib/postgresql/14/bin/pg_ctl``, on Windows there is no + default path), + 2. the directory reported by ``pg_config --bindir``, + 3. on Windows, ``%ProgramFiles%\\PostgreSQL\\*\\bin\\pg_ctl.exe`` (newest version first), + 4. ``pg_ctl`` on ``PATH`` (via ``shutil.which``). + + Only the ``executable`` factory argument is taken at face value; the other locations are + verified to exist before being used. If none of them is there, an ``ExecutableMissingException`` is raised listing the locations that were checked. .. note:: diff --git a/newsfragments/1399.feature.rst b/newsfragments/1399.feature.rst new file mode 100644 index 00000000..2eb8bb83 --- /dev/null +++ b/newsfragments/1399.feature.rst @@ -0,0 +1,4 @@ +``pg_ctl`` discovery now finds a Windows PostgreSQL install without ``--postgresql-exec``. +The default executable path is empty on Windows, and discovery probes +``%ProgramFiles%\\PostgreSQL\\*\\bin\\pg_ctl.exe`` (newest version first) and ``PATH`` after +``pg_config --bindir``. diff --git a/pytest_postgresql/exceptions.py b/pytest_postgresql/exceptions.py index 0b3bf350..e77574e0 100644 --- a/pytest_postgresql/exceptions.py +++ b/pytest_postgresql/exceptions.py @@ -50,6 +50,18 @@ def not_in_bindir(cls, bindir: str, checked: Iterable[str]) -> "ExecutableMissin checked, ) + @classmethod + def discovery_failed(cls, causes: Iterable[str], checked: Iterable[str]) -> "ExecutableMissingException": + """None of the discovery strategies found a runnable pg_ctl. + + :param causes: why each strategy came up empty, joined into one explanation + :param checked: every location that was probed + """ + cause = " ".join(causes) + if not cause: + cause = "No pg_ctl was found in any of the usual locations." + return cls._no_pg_ctl(cause, checked) + class PostgreSQLUnsupported(Exception): """Exception raised when unsupported postgresql would be detected.""" diff --git a/pytest_postgresql/factories/_pg.py b/pytest_postgresql/factories/_pg.py index 8a7b0f1b..0bb354a1 100644 --- a/pytest_postgresql/factories/_pg.py +++ b/pytest_postgresql/factories/_pg.py @@ -3,8 +3,9 @@ import logging import os import platform +import shutil import subprocess -from typing import Iterable +from pathlib import Path from pytest_postgresql.config import PostgreSQLConfig from pytest_postgresql.exceptions import ExecutableMissingException @@ -25,24 +26,28 @@ Matches PostgreSQLExecutor's default subprocess timeout. """ +_WINDOWS_PROGRAM_FILES_FALLBACK = r"C:\Program Files" -def _pg_bindir(checked: Iterable[str]) -> str: + +def _pg_bindir() -> str: """Ask pg_config where PostgreSQL keeps its binaries. pg_config comes from the client development package, which on most distributions can be installed without the server, so its answer is a hint rather than an answer. - :param checked: locations probed so far, to name in the error if pg_config can't be run - :raises ExecutableMissingException: pg_config is missing, unusable, hanging or failing. + :raises OSError: pg_config is missing or not executable + :raises subprocess.SubprocessError: pg_config failed or hung. TimeoutExpired is a SubprocessError, so it needs no branch of its own. """ try: - return subprocess.check_output( + bindir = subprocess.check_output( ["pg_config", "--bindir"], universal_newlines=True, timeout=PG_CONFIG_TIMEOUT ).strip() except (OSError, subprocess.SubprocessError) as ex: logger.debug("Could not read the binaries directory from pg_config: %s", ex) - raise ExecutableMissingException.pg_config_unusable(f"{type(ex).__name__}: {ex}", checked) from ex + raise + logger.debug("pg_config --bindir reported %s", bindir) + return bindir def _is_executable(path: str) -> bool: @@ -54,22 +59,149 @@ def _is_executable(path: str) -> bool: return os.path.isfile(path) and os.access(path, os.X_OK) -def _pg_exe(executable: str | None, config: PostgreSQLConfig) -> str: - """If executable is set, use it. Otherwise best effort to find the executable.""" - # an explicitly passed executable is taken at face value, it's not ours to second-guess - if executable is not None: - return executable +def _windows_postgresql_root() -> Path: + """Directory under which the Windows installer places versioned PostgreSQL copies.""" + program_files = os.environ.get("ProgramFiles", _WINDOWS_PROGRAM_FILES_FALLBACK) + return Path(program_files) / "PostgreSQL" + + +def _windows_version_sort_key(name: str) -> tuple[int, tuple[int, ...] | str]: + """Sort key for Windows PostgreSQL version directory names, newest first when reversed. + + Numeric versions (``16``, ``16.4``) rank above non-numeric names so a real install + wins over stray folders; non-numeric names fall back to lexicographic comparison. + """ + parts = name.split(".") + try: + numbers = tuple(int(part) for part in parts) + except ValueError: + return (0, name) + return (1, numbers) + + +def _windows_program_files_pg_ctls() -> list[str]: + """pg_ctl paths under Program Files/PostgreSQL/*/bin, newest version first.""" + root = _windows_postgresql_root() + if not root.is_dir(): + logger.debug("Windows PostgreSQL directory does not exist: %s", root) + return [] + found: list[tuple[str, str]] = [] + try: + version_dirs = [entry for entry in root.iterdir() if entry.is_dir()] + except OSError as ex: + logger.debug("Could not scan %s: %s", root, ex) + return [] + for version_dir in version_dirs: + bindir = version_dir / "bin" + for name in PG_CTL_NAMES: + candidate = bindir / name + if candidate.is_file(): + found.append((version_dir.name, str(candidate))) + found.sort(key=lambda item: _windows_version_sort_key(item[0]), reverse=True) + paths = [path for _, path in found] + logger.debug("Windows Program Files pg_ctl candidates (newest first): %s", paths) + return paths + + +def _which_pg_ctl() -> str | None: + """Return pg_ctl from PATH, if it is a file this user can run.""" + for name in PG_CTL_NAMES: + found = shutil.which(name) + if found is None: + logger.debug("which(%s) found nothing", name) + continue + logger.debug("which(%s) found %s", name, found) + if _is_executable(found): + return found + return None + + +def _remember(checked: list[str], candidate: str) -> None: + """Append candidate to the probed-locations list unless it is already there.""" + if candidate not in checked: + checked.append(candidate) + + +def _from_config_exec(config: PostgreSQLConfig, checked: list[str]) -> str | None: + """Use postgresql_exec when it names a real executable.""" postgresql_ctl = config.exec - # check if that executable exists, as it's not on systems' PATH + if not postgresql_ctl: + logger.debug("No postgresql_exec configured; skipping that probe") + return None + logger.debug("Checking configured postgresql_exec: %s", postgresql_ctl) if _is_executable(postgresql_ctl): + logger.debug("Using configured postgresql_exec: %s", postgresql_ctl) return postgresql_ctl - checked = [postgresql_ctl] - bindir = _pg_bindir(checked) + _remember(checked, postgresql_ctl) + return None + + +def _from_pg_config(checked: list[str], causes: list[str]) -> str | None: + """Use pg_ctl next to the binaries directory pg_config reports, if any.""" + try: + bindir = _pg_bindir() + except (OSError, subprocess.SubprocessError) as ex: + reason = f"{type(ex).__name__}: {ex}" + logger.debug("pg_config unusable: %s", reason) + causes.append( + f"pg_config could not be run either ({reason}), so it could not be used to locate the PostgreSQL binaries." + ) + return None for name in PG_CTL_NAMES: candidate = os.path.join(bindir, name) - if candidate in checked: - continue - checked.append(candidate) + _remember(checked, candidate) + if _is_executable(candidate): + logger.debug("Using pg_ctl from pg_config bindir: %s", candidate) + return candidate + causes.append( + f"pg_config reports PostgreSQL binaries live in {bindir}, but there's no pg_ctl there. " + f"That usually means only the PostgreSQL client libraries are installed " + f"(Debian/Ubuntu's libpq-dev, for example), without the matching server package." + ) + return None + + +def _from_windows_program_files(checked: list[str]) -> str | None: + """Use the newest Windows installer copy of pg_ctl, if this is Windows.""" + if platform.system() != "Windows": + return None + logger.debug("Probing %s for pg_ctl", _windows_postgresql_root()) + for candidate in _windows_program_files_pg_ctls(): + _remember(checked, candidate) if _is_executable(candidate): + logger.debug("Using pg_ctl from Program Files: %s", candidate) return candidate - raise ExecutableMissingException.not_in_bindir(bindir, checked) + return None + + +def _from_path(checked: list[str]) -> str | None: + """Use pg_ctl from PATH.""" + found = _which_pg_ctl() + if found is None: + logger.debug("shutil.which did not find pg_ctl on PATH") + return None + _remember(checked, found) + if _is_executable(found): + logger.debug("Using pg_ctl from PATH: %s", found) + return found + return None + + +def _pg_exe(executable: str | None, config: PostgreSQLConfig) -> str: + """If executable is set, use it. Otherwise best effort to find the executable.""" + # an explicitly passed executable is taken at face value, it's not ours to second-guess + if executable is not None: + logger.debug("Using factory executable: %s", executable) + return executable + checked: list[str] = [] + causes: list[str] = [] + found = ( + _from_config_exec(config, checked) + or _from_pg_config(checked, causes) + or _from_windows_program_files(checked) + or _from_path(checked) + ) + if found is not None: + return found + logger.debug("pg_ctl discovery failed; checked %s", checked) + raise ExecutableMissingException.discovery_failed(causes, checked) diff --git a/pytest_postgresql/plugin.py b/pytest_postgresql/plugin.py index 47419ec2..5b3159f6 100644 --- a/pytest_postgresql/plugin.py +++ b/pytest_postgresql/plugin.py @@ -37,6 +37,7 @@ pytest_asyncio = None # type: ignore[assignment] _help_executable = "Path to PostgreSQL executable" +_DEFAULT_POSTGRESQL_EXEC = "" if platform.system() == "Windows" else "/usr/lib/postgresql/14/bin/pg_ctl" _help_host = "Host at which PostgreSQL will accept connections" _help_port = "Port at which PostgreSQL will accept connections" _help_port_search_count = "Number of times, pytest-postgresql will search for free port" @@ -144,7 +145,7 @@ def pytest_asyncio_loop_factories( def pytest_addoption(parser: Parser) -> None: """Configure options for pytest-postgresql.""" - parser.addini(name="postgresql_exec", help=_help_executable, default="/usr/lib/postgresql/14/bin/pg_ctl") + parser.addini(name="postgresql_exec", help=_help_executable, default=_DEFAULT_POSTGRESQL_EXEC) parser.addini(name="postgresql_host", help=_help_host, default="127.0.0.1") diff --git a/tests/test_pg_exe.py b/tests/test_pg_exe.py index 37b671aa..006de34f 100644 --- a/tests/test_pg_exe.py +++ b/tests/test_pg_exe.py @@ -2,6 +2,7 @@ import platform import subprocess +from collections.abc import Generator from pathlib import Path from unittest.mock import patch @@ -10,6 +11,7 @@ from pytest_postgresql.config import PostgreSQLConfig from pytest_postgresql.exceptions import ExecutableMissingException from pytest_postgresql.factories import _pg +from pytest_postgresql.plugin import _DEFAULT_POSTGRESQL_EXEC _PG_PROCESS = "pytest_postgresql.factories._pg" @@ -41,6 +43,16 @@ def config(tmp_path: Path) -> PostgreSQLConfig: return make_config(str(tmp_path / "nonexistent" / "bin" / "pg_ctl")) +@pytest.fixture +def isolated_discovery() -> Generator[None, None, None]: + """Disable PATH and Windows Program Files probes so tests isolate earlier strategies.""" + with ( + patch(f"{_PG_PROCESS}.platform.system", return_value="Linux"), + patch.object(_pg, "_which_pg_ctl", return_value=None), + ): + yield + + def make_pg_ctl(bindir: Path, name: str = "pg_ctl", mode: int = 0o755) -> Path: """Create a stand-in for pg_ctl in bindir, executable unless told otherwise.""" bindir.mkdir(parents=True, exist_ok=True) @@ -50,6 +62,14 @@ def make_pg_ctl(bindir: Path, name: str = "pg_ctl", mode: int = 0o755) -> Path: return pg_ctl +def test_postgresql_exec_default_is_platform_aware() -> None: + """Unix keeps the distro path; Windows does not advertise a Linux-only default.""" + if platform.system() == "Windows": + assert _DEFAULT_POSTGRESQL_EXEC == "" + else: + assert _DEFAULT_POSTGRESQL_EXEC == "/usr/lib/postgresql/14/bin/pg_ctl" + + def test_explicit_executable_is_not_second_guessed(config: PostgreSQLConfig) -> None: """An executable passed to the factory is returned even if it does not exist.""" assert _pg._pg_exe("/nowhere/pg_ctl", config) == "/nowhere/pg_ctl" @@ -64,6 +84,18 @@ def test_existing_configured_executable_wins(tmp_path: Path) -> None: bindir_mock.assert_not_called() +@pytest.mark.usefixtures("isolated_discovery") +def test_empty_configured_executable_is_skipped() -> None: + """The Windows default of an empty postgresql_exec is not listed as a checked path.""" + config = make_config("") + with patch.object(_pg, "_pg_bindir", side_effect=FileNotFoundError("pg_config")): + with pytest.raises(ExecutableMissingException) as exc_info: + _pg._pg_exe(None, config) + message = str(exc_info.value) + assert " - \n" not in message + assert "Could not find pg_ctl" in message + + def test_pg_config_bindir_is_used(tmp_path: Path, config: PostgreSQLConfig) -> None: """pg_ctl found next to the binaries pg_config points at is used.""" pg_ctl = make_pg_ctl(tmp_path / "pgconfig-bin") @@ -85,7 +117,73 @@ def test_windows_executable_suffix_is_probed(tmp_path: Path, config: PostgreSQLC assert _pg._pg_exe(None, config) == str(pg_ctl) +def test_windows_program_files_newest_version_wins(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The Windows installer layout is scanned newest version directory first.""" + program_files = tmp_path / "Program Files" + older = make_pg_ctl(program_files / "PostgreSQL" / "14" / "bin", name="pg_ctl.exe") + newest = make_pg_ctl(program_files / "PostgreSQL" / "16" / "bin", name="pg_ctl.exe") + make_pg_ctl(program_files / "PostgreSQL" / "15" / "bin", name="pg_ctl.exe") + monkeypatch.setenv("ProgramFiles", str(program_files)) + with patch.object(_pg, "PG_CTL_NAMES", ("pg_ctl.exe", "pg_ctl")): + found = _pg._windows_program_files_pg_ctls() + assert found[0] == str(newest) + assert str(older) in found + assert found == sorted( + found, + key=lambda path: _pg._windows_version_sort_key(Path(path).parents[1].name), + reverse=True, + ) + + +def test_windows_program_files_missing_root_is_empty(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A missing Program Files PostgreSQL directory yields no candidates.""" + monkeypatch.setenv("ProgramFiles", str(tmp_path / "no-such-dir")) + assert _pg._windows_program_files_pg_ctls() == [] + + +def test_windows_program_files_empty_tree_is_empty(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """An empty PostgreSQL directory under Program Files yields no candidates.""" + (tmp_path / "PostgreSQL").mkdir() + monkeypatch.setenv("ProgramFiles", str(tmp_path)) + with patch.object(_pg, "PG_CTL_NAMES", ("pg_ctl.exe", "pg_ctl")): + assert _pg._windows_program_files_pg_ctls() == [] + + +def test_pg_exe_uses_windows_program_files(tmp_path: Path, config: PostgreSQLConfig) -> None: + """When pg_config fails, a Program Files install is used on Windows.""" + pg_ctl = make_pg_ctl(tmp_path / "bin", name="pg_ctl.exe") + with ( + patch(f"{_PG_PROCESS}.platform.system", return_value="Windows"), + patch.object(_pg, "_pg_bindir", side_effect=FileNotFoundError("pg_config")), + patch.object(_pg, "_windows_program_files_pg_ctls", return_value=[str(pg_ctl)]), + patch.object(_pg, "_which_pg_ctl", return_value=None), + ): + assert _pg._pg_exe(None, config) == str(pg_ctl) + + +def test_pg_exe_uses_which_when_other_probes_miss(tmp_path: Path, config: PostgreSQLConfig) -> None: + """pg_ctl on PATH is used after pg_config and Program Files come up empty.""" + pg_ctl = make_pg_ctl(tmp_path / "path-bin") + with ( + patch(f"{_PG_PROCESS}.platform.system", return_value="Linux"), + patch.object(_pg, "_pg_bindir", side_effect=FileNotFoundError("pg_config")), + patch.object(_pg, "_which_pg_ctl", return_value=str(pg_ctl)), + ): + assert _pg._pg_exe(None, config) == str(pg_ctl) + + +def test_which_pg_ctl_returns_first_executable_on_path(tmp_path: Path) -> None: + """_which_pg_ctl asks shutil.which for each PG_CTL_NAMES entry.""" + pg_ctl = make_pg_ctl(tmp_path / "path-bin") + with ( + patch.object(_pg, "PG_CTL_NAMES", ("pg_ctl.exe", "pg_ctl")), + patch(f"{_PG_PROCESS}.shutil.which", side_effect=[None, str(pg_ctl)]), + ): + assert _pg._which_pg_ctl() == str(pg_ctl) + + @pytest.mark.skipif(platform.system() == "Windows", reason="os.access ignores X_OK on Windows") +@pytest.mark.usefixtures("isolated_discovery") def test_non_executable_configured_path_is_rejected(tmp_path: Path) -> None: """A configured pg_ctl that exists but cannot be run is not handed back. @@ -99,6 +197,7 @@ def test_non_executable_configured_path_is_rejected(tmp_path: Path) -> None: @pytest.mark.skipif(platform.system() == "Windows", reason="os.access ignores X_OK on Windows") +@pytest.mark.usefixtures("isolated_discovery") def test_non_executable_discovered_path_is_rejected(tmp_path: Path, config: PostgreSQLConfig) -> None: """A pg_ctl in pg_config's bindir that cannot be run is not handed back either.""" pg_ctl = make_pg_ctl(tmp_path / "pgconfig-bin", mode=0o644) @@ -109,6 +208,7 @@ def test_non_executable_discovered_path_is_rejected(tmp_path: Path, config: Post assert str(pg_ctl) in str(exc_info.value) +@pytest.mark.usefixtures("isolated_discovery") def test_client_only_install_raises_early(tmp_path: Path, config: PostgreSQLConfig) -> None: """A pg_config without a matching server fails at discovery time, not at version check. @@ -136,6 +236,7 @@ def test_client_only_install_raises_early(tmp_path: Path, config: PostgreSQLConf subprocess.TimeoutExpired("pg_config", _pg.PG_CONFIG_TIMEOUT), ], ) +@pytest.mark.usefixtures("isolated_discovery") def test_broken_pg_config_surfaces_as_executable_missing(config: PostgreSQLConfig, error: Exception) -> None: """A missing, unusable, hanging or failing pg_config is reported as a missing executable. @@ -156,4 +257,4 @@ def test_broken_pg_config_surfaces_as_executable_missing(config: PostgreSQLConfi def test_pg_bindir_returns_stripped_output() -> None: """The binaries directory is read from pg_config without surrounding whitespace.""" with patch(f"{_PG_PROCESS}.subprocess.check_output", return_value="/usr/lib/postgresql/17/bin\n"): - assert _pg._pg_bindir([]) == "/usr/lib/postgresql/17/bin" + assert _pg._pg_bindir() == "/usr/lib/postgresql/17/bin" diff --git a/tests/test_windows_compatibility.py b/tests/test_windows_compatibility.py index 2b15cd99..4d00f6d8 100644 --- a/tests/test_windows_compatibility.py +++ b/tests/test_windows_compatibility.py @@ -660,7 +660,7 @@ def test_stop_method_fallback_on_killpg_error(self) -> None: result = executor.stop() finally: if real_killpg is not None: - os.killpg = real_killpg + setattr(os, "killpg", real_killpg) # Should call pg_ctl stop, fail on super().stop, then use Windows terminate mock_subprocess.assert_called_once() From 1a1ebae563cceae53c916bea2560c496ec24c0eb Mon Sep 17 00:00:00 2001 From: tboy1337 Date: Tue, 1 Sep 2026 19:30:22 +0700 Subject: [PATCH 2/4] Cover remaining pg_ctl discovery branches for patch coverage. --- tests/test_pg_exe.py | 73 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/tests/test_pg_exe.py b/tests/test_pg_exe.py index 006de34f..3e0de4f1 100644 --- a/tests/test_pg_exe.py +++ b/tests/test_pg_exe.py @@ -4,7 +4,7 @@ import subprocess from collections.abc import Generator from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -149,6 +149,39 @@ def test_windows_program_files_empty_tree_is_empty(tmp_path: Path, monkeypatch: assert _pg._windows_program_files_pg_ctls() == [] +def test_windows_numeric_version_outranks_stray_folder(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Non-numeric Program Files folders sort after real version directories.""" + program_files = tmp_path / "Program Files" + stray = make_pg_ctl(program_files / "PostgreSQL" / "extra" / "bin", name="pg_ctl.exe") + numeric = make_pg_ctl(program_files / "PostgreSQL" / "16" / "bin", name="pg_ctl.exe") + monkeypatch.setenv("ProgramFiles", str(program_files)) + with patch.object(_pg, "PG_CTL_NAMES", ("pg_ctl.exe", "pg_ctl")): + found = _pg._windows_program_files_pg_ctls() + assert found[0] == str(numeric) + assert str(stray) in found + + +def test_windows_program_files_scan_oserror_returns_empty() -> None: + """A Program Files tree that cannot be listed is treated as empty, not a crash.""" + fake_root = MagicMock() + fake_root.is_dir.return_value = True + fake_root.iterdir.side_effect = OSError("permission denied") + with patch.object(_pg, "_windows_postgresql_root", return_value=fake_root): + assert _pg._windows_program_files_pg_ctls() == [] + + +def test_windows_postgresql_root_uses_fallback_without_env(monkeypatch: pytest.MonkeyPatch) -> None: + """When ProgramFiles is unset, the conventional Program Files location is used.""" + monkeypatch.delenv("ProgramFiles", raising=False) + assert _pg._windows_postgresql_root() == Path(r"C:\Program Files") / "PostgreSQL" + + +def test_discovery_failed_without_causes_still_explains() -> None: + """A discovery failure with no strategy-specific cause still tells the reader what happened.""" + exc = ExecutableMissingException.discovery_failed([], []) + assert "No pg_ctl was found in any of the usual locations." in str(exc) + + def test_pg_exe_uses_windows_program_files(tmp_path: Path, config: PostgreSQLConfig) -> None: """When pg_config fails, a Program Files install is used on Windows.""" pg_ctl = make_pg_ctl(tmp_path / "bin", name="pg_ctl.exe") @@ -182,6 +215,44 @@ def test_which_pg_ctl_returns_first_executable_on_path(tmp_path: Path) -> None: assert _pg._which_pg_ctl() == str(pg_ctl) +@pytest.mark.skipif(platform.system() == "Windows", reason="os.access ignores X_OK on Windows") +def test_which_pg_ctl_skips_non_executable(tmp_path: Path) -> None: + """A PATH hit that cannot be executed is not returned.""" + pg_ctl = make_pg_ctl(tmp_path / "path-bin", mode=0o644) + with ( + patch.object(_pg, "PG_CTL_NAMES", ("pg_ctl",)), + patch(f"{_PG_PROCESS}.shutil.which", return_value=str(pg_ctl)), + ): + assert _pg._which_pg_ctl() is None + + +@pytest.mark.skipif(platform.system() == "Windows", reason="os.access ignores X_OK on Windows") +def test_pg_exe_rejects_non_executable_program_files_hit(tmp_path: Path, config: PostgreSQLConfig) -> None: + """A Program Files pg_ctl that exists but cannot be run is skipped, then discovery fails.""" + pg_ctl = make_pg_ctl(tmp_path / "bin", mode=0o644) + with ( + patch(f"{_PG_PROCESS}.platform.system", return_value="Windows"), + patch.object(_pg, "_pg_bindir", side_effect=FileNotFoundError("pg_config")), + patch.object(_pg, "_windows_program_files_pg_ctls", return_value=[str(pg_ctl)]), + patch.object(_pg, "_which_pg_ctl", return_value=None), + ): + with pytest.raises(ExecutableMissingException, match="Could not find pg_ctl"): + _pg._pg_exe(None, config) + + +@pytest.mark.skipif(platform.system() == "Windows", reason="os.access ignores X_OK on Windows") +def test_pg_exe_rejects_non_executable_which_hit(tmp_path: Path, config: PostgreSQLConfig) -> None: + """A PATH pg_ctl that cannot be run is not handed back either.""" + pg_ctl = make_pg_ctl(tmp_path / "path-bin", mode=0o644) + with ( + patch(f"{_PG_PROCESS}.platform.system", return_value="Linux"), + patch.object(_pg, "_pg_bindir", side_effect=FileNotFoundError("pg_config")), + patch.object(_pg, "_which_pg_ctl", return_value=str(pg_ctl)), + ): + with pytest.raises(ExecutableMissingException, match="Could not find pg_ctl"): + _pg._pg_exe(None, config) + + @pytest.mark.skipif(platform.system() == "Windows", reason="os.access ignores X_OK on Windows") @pytest.mark.usefixtures("isolated_discovery") def test_non_executable_configured_path_is_rejected(tmp_path: Path) -> None: From ebd506cd752e764bceaefcfe91b419de1a2287e7 Mon Sep 17 00:00:00 2001 From: tboy1337 Date: Tue, 1 Sep 2026 20:03:54 +0700 Subject: [PATCH 3/4] Use single backslashes in RST Windows path literals so rendered docs show the real Program Files path. --- README.rst | 2 +- newsfragments/1399.feature.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 3e80cd8d..4615139f 100644 --- a/README.rst +++ b/README.rst @@ -447,7 +447,7 @@ You can define settings via fixture factory arguments, command line options, or on Unix this defaults to ``/usr/lib/postgresql/14/bin/pg_ctl``, on Windows there is no default path), 2. the directory reported by ``pg_config --bindir``, - 3. on Windows, ``%ProgramFiles%\\PostgreSQL\\*\\bin\\pg_ctl.exe`` (newest version first), + 3. on Windows, ``%ProgramFiles%\PostgreSQL\*\bin\pg_ctl.exe`` (newest version first), 4. ``pg_ctl`` on ``PATH`` (via ``shutil.which``). Only the ``executable`` factory argument is taken at face value; the other locations are diff --git a/newsfragments/1399.feature.rst b/newsfragments/1399.feature.rst index 2eb8bb83..33cc2339 100644 --- a/newsfragments/1399.feature.rst +++ b/newsfragments/1399.feature.rst @@ -1,4 +1,4 @@ ``pg_ctl`` discovery now finds a Windows PostgreSQL install without ``--postgresql-exec``. The default executable path is empty on Windows, and discovery probes -``%ProgramFiles%\\PostgreSQL\\*\\bin\\pg_ctl.exe`` (newest version first) and ``PATH`` after +``%ProgramFiles%\PostgreSQL\*\bin\pg_ctl.exe`` (newest version first) and ``PATH`` after ``pg_config --bindir``. From 292d3251af1ed17c22266b738326e9871a9bb7d0 Mon Sep 17 00:00:00 2001 From: tboy1337 Date: Wed, 2 Sep 2026 05:31:00 +0700 Subject: [PATCH 4/4] Drop the _pg_exe comment that restates the executable check. --- pytest_postgresql/factories/_pg.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pytest_postgresql/factories/_pg.py b/pytest_postgresql/factories/_pg.py index 0bb354a1..961148c7 100644 --- a/pytest_postgresql/factories/_pg.py +++ b/pytest_postgresql/factories/_pg.py @@ -189,7 +189,6 @@ def _from_path(checked: list[str]) -> str | None: def _pg_exe(executable: str | None, config: PostgreSQLConfig) -> str: """If executable is set, use it. Otherwise best effort to find the executable.""" - # an explicitly passed executable is taken at face value, it's not ours to second-guess if executable is not None: logger.debug("Using factory executable: %s", executable) return executable