Cleaner error message when the server is not installed - closes #1031

README.rst

@@ -440,7 +440,22 @@ You can define settings via fixture factory arguments, command line options, or
 
 .. note::
 
-    If the ``executable`` is not provided, the plugin attempts to find it by calling ``pg_config``. If that fails, it falls back to a common path like ``/usr/lib/postgresql/14/bin/pg_ctl``.
+    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``.
+
+    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``
+    is raised listing the locations that were checked.
+
+.. note::
+
+    ``postgresql_proc`` starts and manages a PostgreSQL server for you, so it needs the
+    PostgreSQL **server** installed locally. Installing only the client libraries
+    (``libpq-dev`` on Debian/Ubuntu) provides ``pg_config`` but no ``pg_ctl``, and the plugin
+    will refuse to start. To test against a server you run yourself — a dockerised one, for
+    instance — use the ``postgresql_noproc`` fixture instead.
 
 Examples
 ========

newsfragments/1031.bugfix.rst

@@ -0,0 +1,10 @@
+``pg_ctl`` discovery no longer hands back a path that does not exist. A client-only PostgreSQL
+install (Debian/Ubuntu's ``libpq-dev`` without the matching server package) makes
+``pg_config --bindir`` point at a directory holding no ``pg_ctl``; that path used to be returned
+unchecked, and the failure only surfaced later as ``Could not found ...`` when the executor read the
+server version. The path is now verified up front, and the ``ExecutableMissingException`` names the
+locations that were checked along with the ways to fix it: install the server package, point at a
+``pg_ctl`` with ``--postgresql-exec``, or use ``postgresql_noproc`` against a server you run yourself.
+
+A broken ``pg_config`` is reported the same way. Only ``FileNotFoundError`` used to be handled, so a
+non-executable or failing ``pg_config`` escaped as a raw ``PermissionError``/``CalledProcessError``.

newsfragments/1031.docs.rst

@@ -0,0 +1,3 @@
+Corrected the README's description of how ``pg_ctl`` is located — it listed the discovery steps in
+the wrong order — and spelled out that ``postgresql_proc`` needs a local PostgreSQL server, pointing
+at ``postgresql_noproc`` for dockerised or otherwise externally managed servers.

pytest_postgresql/exceptions.py

@@ -1,8 +1,54 @@
 """pytest-postgresql's exceptions."""
 
+from typing import Iterable
+
 
 class ExecutableMissingException(FileNotFoundError):
-    """Exception risen when PgConfig was not found."""
+    """Exception raised when pg_ctl, needed to start a PostgreSQL server, was not found."""
+
+    _REMEDIES = (
+        "To fix this, either:\n"
+        "  - install the PostgreSQL server package "
+        "(apt install postgresql, dnf install postgresql-server, brew install postgresql),\n"
+        "  - point the plugin at an existing pg_ctl with the --postgresql-exec command line option, "
+        "the postgresql_exec ini option, or the executable factory argument,\n"
+        "  - or use the postgresql_noproc fixture to connect to a server you run yourself, "
+        "a dockerised one for instance."
+    )
+
+    @classmethod
+    def _no_pg_ctl(cls, cause: str, checked: Iterable[str]) -> "ExecutableMissingException":
+        """Report the locations that were probed, why they came up empty, and what to do."""
+        locations = "\n".join(f"  - {location}" for location in checked)
+        return cls(
+            f"Could not find pg_ctl, which is needed to start a PostgreSQL server.\n"
+            f"{cause}\n"
+            f"Checked:\n{locations}\n"
+            f"{cls._REMEDIES}"
+        )
+
+    @classmethod
+    def pg_config_unusable(cls, reason: str, checked: Iterable[str]) -> "ExecutableMissingException":
+        """pg_config is missing, not executable, hanging or failing, so it cannot be asked.
+
+        :param reason: concise description of how running pg_config failed, so the reader
+            can tell "not installed" from "not executable" from "timed out"
+        :param checked: locations probed so far
+        """
+        return cls._no_pg_ctl(
+            f"pg_config could not be run either ({reason}), so it could not be used to locate the PostgreSQL binaries.",
+            checked,
+        )
+
+    @classmethod
+    def not_in_bindir(cls, bindir: str, checked: Iterable[str]) -> "ExecutableMissingException":
+        """pg_config named a binaries directory, but no pg_ctl lives there."""
+        return cls._no_pg_ctl(
+            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.",
+            checked,
+        )
 
 
 class PostgreSQLUnsupported(Exception):

pytest_postgresql/executor.py

@@ -342,11 +342,12 @@ def version(self) -> Any:
         """Detect postgresql version."""
         try:
             version_string = subprocess.check_output([self.executable, "--version"]).decode("utf-8")
-        except FileNotFoundError as ex:
+        except OSError as ex:
             raise ExecutableMissingException(
-                f"Could not find {self.executable}. Is PostgreSQL server installed? "
-                f"Alternatively pg_config installed might be from different "
-                f"version that postgresql-server."
+                f"Could not run {self.executable} to read the PostgreSQL version: {ex}. "
+                f"Is the PostgreSQL server installed at that location? "
+                f"Point the plugin at a working pg_ctl with the --postgresql-exec command line option, "
+                f"the postgresql_exec ini option, or the executable factory argument."
             ) from ex
         matches = self.VERSION_RE.search(version_string)
         assert matches is not None

pytest_postgresql/factories/_pg.py

@@ -0,0 +1,75 @@
+"""Internal module for functions handling pg_ctl discovery."""
+
+import logging
+import os
+import platform
+import subprocess
+from typing import Iterable
+
+from pytest_postgresql.config import PostgreSQLConfig
+from pytest_postgresql.exceptions import ExecutableMissingException
+
+logger = logging.getLogger(__name__)
+
+PG_CTL_NAMES = ("pg_ctl.exe", "pg_ctl") if platform.system() == "Windows" else ("pg_ctl",)
+"""Names to look for when probing the filesystem for pg_ctl.
+
+os.path.isfile matches the literal name, so finding the binary on Windows means asking
+for pg_ctl.exe. Launching it is more forgiving: Windows appends .exe to an extensionless
+program name, which is why the old code could run a path it never checked.
+"""
+
+PG_CONFIG_TIMEOUT = 60
+"""Seconds to wait for pg_config, so a hung binary cannot stall fixture setup.
+
+Matches PostgreSQLExecutor's default subprocess timeout.
+"""
+
+
+def _pg_bindir(checked: Iterable[str]) -> 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.
+        TimeoutExpired is a SubprocessError, so it needs no branch of its own.
+    """
+    try:
+        return 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
+
+
+def _is_executable(path: str) -> bool:
+    """Whether path is a file this user can actually run.
+
+    isfile is needed alongside the access check because X_OK on a directory only means
+    it can be traversed. On Windows os.access ignores X_OK, so this is an existence test.
+    """
+    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
+    postgresql_ctl = config.exec
+    # check if that executable exists, as it's not on systems' PATH
+    if _is_executable(postgresql_ctl):
+        return postgresql_ctl
+    checked = [postgresql_ctl]
+    bindir = _pg_bindir(checked)
+    for name in PG_CTL_NAMES:
+        candidate = os.path.join(bindir, name)
+        if candidate in checked:
+            continue
+        checked.append(candidate)
+        if _is_executable(candidate):
+            return candidate
+    raise ExecutableMissingException.not_in_bindir(bindir, checked)

pytest_postgresql/factories/process.py

@@ -21,7 +21,6 @@
 import os
 import os.path
 import platform
-import subprocess
 import tempfile
 from pathlib import Path
 from typing import Callable, Iterable
@@ -32,29 +31,15 @@
 from pytest import FixtureRequest, TempPathFactory
 
 from pytest_postgresql.config import PostgreSQLConfig, get_config
-from pytest_postgresql.exceptions import ExecutableMissingException
 from pytest_postgresql.executor import PostgreSQLExecutor
+from pytest_postgresql.factories._pg import _pg_exe
 from pytest_postgresql.janitor import DatabaseJanitor
 
 logger = logging.getLogger(__name__)
 
 PortType = port_for.PortType  # mypy requires explicit export
 
 
-def _pg_exe(executable: str | None, config: PostgreSQLConfig) -> str:
-    """If executable is set, use it. Otherwise best effort to find the executable."""
-    postgresql_ctl = executable or config.exec
-    # check if that executable exists, as it's no on systems' PATH
-    # only replace it if executable isn't passed manually
-    if not os.path.exists(postgresql_ctl) and executable is None:
-        try:
-            pg_bindir = subprocess.check_output(["pg_config", "--bindir"], universal_newlines=True).strip()
-        except FileNotFoundError as ex:
-            raise ExecutableMissingException("Could not find pg_config executable. Is it in system $PATH?") from ex
-        postgresql_ctl = os.path.join(pg_bindir, "pg_ctl")
-    return postgresql_ctl
-
-
 def _pg_port(port: PortType | None, config: PostgreSQLConfig, excluded_ports: Iterable[int]) -> int:
     """User specified port, otherwise find an unused port from config."""
     pg_port = get_port(port, excluded_ports) or get_port(config.port, excluded_ports)

tests/test_executor.py

@@ -20,7 +20,7 @@
 from pytest_postgresql.config import get_config
 from pytest_postgresql.exceptions import PostgreSQLUnsupported
 from pytest_postgresql.executor import PostgreSQLExecutor
-from pytest_postgresql.factories import postgresql, postgresql_async, postgresql_proc
+from pytest_postgresql.factories import _pg, postgresql, postgresql_async, postgresql_proc
 from pytest_postgresql.retry import retry
 
 
@@ -250,7 +250,7 @@ def test_executor_init_with_password(
     """Test whether the executor initializes properly."""
     config = get_config(request)
     monkeypatch.setenv("LC_ALL", locale)
-    pg_exe = process._pg_exe(None, config)
+    pg_exe = _pg._pg_exe(None, config)
     port = process._pg_port(None, config, [])
     tmpdir = tmp_path_factory.mktemp(f"pytest-postgresql-{request.node.name}")
     datadir, logfile_path = process._prepare_dir(tmpdir, port, "test")
@@ -274,7 +274,7 @@ def test_executor_init_bad_tmp_path(
 ) -> None:
     r"""Test init with \ and space chars in the path."""
     config = get_config(request)
-    pg_exe = process._pg_exe(None, config)
+    pg_exe = _pg._pg_exe(None, config)
     port = process._pg_port(None, config, [])
     tmpdir = tmp_path_factory.mktemp(f"pytest-postgresql-{request.node.name}") / r"a bad\path/"
     tmpdir.mkdir(parents=True, exist_ok=True)
@@ -320,7 +320,9 @@ def test_executor_platform_template_selection(
     command template based on the platform.
     """
     config = get_config(request)
-    pg_exe = process._pg_exe(None, config)
+    # The executor is never started here, only its command inspected, so an arbitrary
+    # path will do - discovering a real pg_ctl would tie this to an installed server.
+    pg_exe = "/usr/bin/pg_ctl"
     port = process._pg_port(-1, config, [])
     tmpdir = tmp_path_factory.mktemp(f"pytest-postgresql-{request.node.name}")
     datadir, logfile_path = process._prepare_dir(tmpdir, port, "test")
@@ -359,7 +361,7 @@ def test_executor_with_special_chars_in_all_paths(
     postgres_options all at the same time.
     """
     config = get_config(request)
-    pg_exe = process._pg_exe(None, config)
+    pg_exe = _pg._pg_exe(None, config)
     port = process._pg_port(None, config, [])
     # Create a tmpdir with spaces in the name
     tmpdir = tmp_path_factory.mktemp(f"pytest-postgresql-{request.node.name}") / "my test dir"
@@ -772,7 +774,7 @@ def test_actual_postgresql_start_windows(
     correctly starts PostgreSQL on actual Windows systems.
     """
     config = get_config(request)
-    pg_exe = process._pg_exe(None, config)
+    pg_exe = _pg._pg_exe(None, config)
     port = process._pg_port(None, config, [])
     tmpdir = tmp_path_factory.mktemp(f"pytest-postgresql-{request.node.name}")
     datadir, logfile_path = process._prepare_dir(tmpdir, port, "test")
@@ -811,7 +813,7 @@ def test_actual_postgresql_start_unix(
     correctly starts PostgreSQL on actual Unix/Linux systems.
     """
     config = get_config(request)
-    pg_exe = process._pg_exe(None, config)
+    pg_exe = _pg._pg_exe(None, config)
     port = process._pg_port(None, config, [])
     tmpdir = tmp_path_factory.mktemp(f"pytest-postgresql-{request.node.name}")
     datadir, logfile_path = process._prepare_dir(tmpdir, port, "test")
@@ -847,7 +849,7 @@ def test_actual_postgresql_start_darwin(
     PostgreSQL on actual Darwin/macOS systems and uses the correct locale.
     """
     config = get_config(request)
-    pg_exe = process._pg_exe(None, config)
+    pg_exe = _pg._pg_exe(None, config)
     port = process._pg_port(None, config, [])
     tmpdir = tmp_path_factory.mktemp(f"pytest-postgresql-{request.node.name}")
     datadir, logfile_path = process._prepare_dir(tmpdir, port, "test")