Skip to content

Commit 7f7914b

Browse files
committed
Refactor Windows command handling in PostgreSQLExecutor
- Introduced a new static method _windows_pg_options to centralize the construction of the -o argument for the Windows pg_ctl start invocation, ensuring consistent command arguments across initialization and start methods. - Updated the command template in the Windows start branch to utilize the new helper method, improving clarity and maintainability. - Enhanced tests to verify that the command template and generated options do not include single quotes, aligning with Windows command requirements.
1 parent c96148a commit 7f7914b

2 files changed

Lines changed: 43 additions & 24 deletions

File tree

pytest_postgresql/executor.py

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,27 @@ class PostgreSQLExecutor(TCPExecutor):
7171

7272
# Windows command template - no single quotes (cmd.exe treats them as literals,
7373
# not delimiters) and unix_socket_directories is omitted entirely since PostgreSQL
74-
# ignores it on Windows. On Windows, mirakuru forces shell=True so the command
75-
# goes through cmd.exe.
74+
# ignores it on Windows. The -o payload is produced by _windows_pg_options() so
75+
# both __init__ and start() always use identical arguments.
7676
WINDOWS_PROC_START_COMMAND = (
7777
'"{executable}" start -D "{datadir}" '
78-
'-o "-F -p {port} -c log_destination=stderr '
79-
'-c logging_collector=off{postgres_options}" '
78+
'-o "{pg_options}" '
8079
'-l "{logfile}" {startparams}'
8180
)
8281

8382
VERSION_RE = re.compile(r".* (?P<version>\d+(?:\.\d+)?)")
8483
MIN_SUPPORTED_VERSION = parse("14")
8584

85+
@staticmethod
86+
def _windows_pg_options(port: int, postgres_options: str) -> str:
87+
"""Return the -o argument value for the Windows pg_ctl start invocation.
88+
89+
Centralises the construction so __init__ (command string) and start()
90+
(argv list) always produce identical payloads without risk of drift.
91+
"""
92+
extra = f" {postgres_options}" if postgres_options else ""
93+
return f"-F -p {port} -c log_destination=stderr -c logging_collector=off{extra}"
94+
8695
def __init__(
8796
self,
8897
executable: str,
@@ -133,22 +142,27 @@ def __init__(
133142
self.startparams = startparams
134143
self.postgres_options = postgres_options
135144
if platform.system() == "Windows":
136-
command_template = self.WINDOWS_PROC_START_COMMAND
145+
command = self.WINDOWS_PROC_START_COMMAND.format(
146+
executable=self.executable,
147+
datadir=self.datadir,
148+
pg_options=PostgreSQLExecutor._windows_pg_options(port, self.postgres_options),
149+
logfile=self.logfile,
150+
startparams=self.startparams,
151+
)
137152
else:
138-
command_template = self.UNIX_PROC_START_COMMAND
139-
# PostgreSQL GUC single-quoted strings double single-quotes to escape them
140-
# (e.g. /tmp/o'hare → /tmp/o''hare). Apply this before interpolation so
141-
# the generated unix_socket_directories value is always syntactically valid.
142-
escaped_unixsocketdir = self.unixsocketdir.replace("'", "''")
143-
command = command_template.format(
144-
executable=self.executable,
145-
datadir=self.datadir,
146-
port=port,
147-
unixsocketdir=escaped_unixsocketdir,
148-
logfile=self.logfile,
149-
startparams=self.startparams,
150-
postgres_options=f" {self.postgres_options}" if self.postgres_options else "",
151-
)
153+
# PostgreSQL GUC single-quoted strings double single-quotes to escape them
154+
# (e.g. /tmp/o'hare → /tmp/o''hare). Apply this before interpolation so
155+
# the generated unix_socket_directories value is always syntactically valid.
156+
escaped_unixsocketdir = self.unixsocketdir.replace("'", "''")
157+
command = self.UNIX_PROC_START_COMMAND.format(
158+
executable=self.executable,
159+
datadir=self.datadir,
160+
port=port,
161+
unixsocketdir=escaped_unixsocketdir,
162+
logfile=self.logfile,
163+
startparams=self.startparams,
164+
postgres_options=f" {self.postgres_options}" if self.postgres_options else "",
165+
)
152166
super().__init__(
153167
command,
154168
host,
@@ -186,8 +200,7 @@ def start(self: T) -> T:
186200
# so the loop never sees a live process and times out.
187201
# Run pg_ctl synchronously as an argv list (no shell) so quoting
188202
# is handled safely by subprocess, mirroring the stop() approach.
189-
postgres_options_str = f" {self.postgres_options}" if self.postgres_options else ""
190-
pg_options = f"-F -p {self.port} -c log_destination=stderr -c logging_collector=off{postgres_options_str}"
203+
pg_options = self._windows_pg_options(self.port, self.postgres_options)
191204
args = [
192205
self.executable,
193206
"start",

tests/test_windows_compatibility.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,16 @@ def test_windows_command_template_no_single_quotes(self) -> None:
3131
"""
3232
template = PostgreSQLExecutor.WINDOWS_PROC_START_COMMAND
3333

34-
# Windows template should NOT use single quotes
35-
assert "log_destination=stderr" in template
36-
assert "log_destination='stderr'" not in template
34+
# Template itself must contain no single quotes and delegate the -o payload
35+
# to the _windows_pg_options helper (tested below).
3736
assert "'" not in template
37+
assert "log_destination='stderr'" not in template
38+
39+
# The -o payload produced by the helper must use bare stderr (no single quotes)
40+
# because cmd.exe treats single quotes as literal characters.
41+
pg_options = PostgreSQLExecutor._windows_pg_options(5432, "")
42+
assert "log_destination=stderr" in pg_options
43+
assert "'" not in pg_options
3844

3945
def test_windows_command_template_omits_unix_socket_directories(self) -> None:
4046
"""Test that Windows template does not include unix_socket_directories.

0 commit comments

Comments
 (0)