Commit d912c6a
![pre-commit-ci[bot]](https://avatars.githubusercontent.com/in/68672?v=4&size=40){kind=avatar}
Refactor PostgreSQLExecutor to support Windows compatibility for process management (#1182)
* Refactor PostgreSQLExecutor to support Windows compatibility for process management
- Convert _get_base_command method to BASE_PROC_START_COMMAND class attribute
- Use unified command format without single quotes around PostgreSQL config values
- Add _windows_terminate_process method for graceful Windows process termination
- Update stop() method to use list args instead of shell=True for security
- Add platform-specific termination logic with killpg fallback
- Add comprehensive Windows compatibility test suite
- Rename parameter in _windows_terminate_process method for clarity
* Enhance PostgreSQL workflow for Windows compatibility and streamline environment variable usage
- Added detection of PostgreSQL path for Windows runners.
- Set PostgreSQL path for Unix/macOS environments.
- Updated test commands to utilize the dynamically set PostgreSQL executable path.
- Introduced new Windows job configurations for PostgreSQL versions 16, 17, and 18 in tests.yml.
- Removed unnecessary import in conftest.py as the plugin is registered via entry point.
* Enhance PostgreSQL workflow and executor for improved cross-platform compatibility
- Implement dynamic detection of pg_ctl path for Unix/macOS environments in the GitHub Actions workflow.
- Add logging for exception handling during Windows process termination in PostgreSQLExecutor.
- Update test mocks to reference the correct subprocess path for better compatibility.
- Remove unnecessary import in conftest.py to streamline the codebase.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Refactor PostgreSQLExecutor and enhance Windows compatibility in tests
- Reintroduce logger initialization in executor.py for improved logging.
- Remove unnecessary pass statement in exception handling within the stop method.
- Update test assertions to ensure correct subprocess call format for Windows compatibility.
- Temporarily modify os.killpg attribute in tests to validate fallback behavior on Windows termination.
* Improve process termination handling in PostgreSQLExecutor and refine test assertions
- Add timeout handling for process cleanup in the stop method to prevent zombie processes.
- Update test to assert correct argument usage for mock_terminate in Windows compatibility tests.
* Update GitHub workflows to include editable package installation
- Added steps to install the package in editable mode before running tests in dockerised-postgres.yml, oldest-postgres.yml, and single-postgres.yml.
- Streamlined the testing process by ensuring the package is available in the current environment for all workflows.
* Update oldest-postgres.yml to install package without dependencies
- Modified the package installation command to include the --no-deps flag, ensuring that only the editable package is installed without its dependencies before running tests.
* Enhance PostgreSQL workflow error handling
- Added verification step to ensure pg_ctl is found in expected locations during the GitHub Actions workflow for single-postgres.yml.
- Implemented error logging and exit strategy if PostgreSQL is not detected, improving robustness of the setup process.
* Refactor PostgreSQLExecutor command templates for platform compatibility
- Introduced separate command templates for Unix and Windows to handle quoting and configuration differences.
- Updated tests to validate the correct command template is used based on the operating system.
- Enhanced test coverage for command formatting, ensuring proper handling of paths with spaces on Unix and omission of unnecessary parameters on Windows.
* Fix PostgreSQL path in Windows workflow
- Updated the pg_ctl path in single-postgres.yml to include the .exe extension for compatibility with Windows environments, ensuring proper execution of PostgreSQL commands.
* Update pytest configuration in test_postgres_options_plugin.py
- Changed the conftest file generation to use a more explicit pytest_plugins declaration for better clarity and compatibility with pytest's plugin system.
* Update pytest_plugins declaration in test_postgres_options_plugin.py for improved clarity
- Changed the pytest_plugins declaration in the conftest file generation to specify the plugin path explicitly, enhancing compatibility with pytest's plugin system.
* Enhance platform-specific command templates and test coverage for PostgreSQLExecutor
- Added tests to verify correct command templates for Windows, Unix, and Darwin platforms, ensuring proper handling of `unix_socket_directories` and `log_destination`.
- Implemented checks for locale settings on Darwin and preserved quoting for `postgres_options` across different platforms.
- Improved test assertions for command generation with special characters and empty parameters, enhancing overall test robustness.
* Add Windows locale setup fixture and update test cases for password handling
- Introduced a pytest fixture to set Windows-compatible locale environment variables, ensuring compatibility with PostgreSQL's initdb on Windows.
- Updated test cases for PostgreSQL start commands across Windows, Unix, and Darwin platforms to include a password parameter, enhancing test coverage and consistency.
* Update locale handling in executor.py and remove Windows locale setup fixture from conftest.py
- Modified locale settings in executor.py to ensure compatibility across Darwin and Windows platforms.
- Removed the Windows locale setup fixture from conftest.py as the locale handling is now managed directly in executor.py, simplifying the test configuration.
* Refactor socket directory handling in test_executor.py for PostgreSQLExecutor
- Updated the test case to create a dedicated socket directory for Unix systems, improving clarity and organization in the test setup.
- Adjusted the path for the `unixsocketdir` parameter to use the newly created socket directory, ensuring proper configuration for PostgreSQLExecutor.
* Update path handling for pytest uploads in single-postgres.yml
- Modified the upload path for pytest results to include both the temporary runner directory and a specific /tmp directory, ensuring comprehensive coverage of test artifacts during the workflow execution.
* Update pytest command options in single-postgres.yml to include --basetemp for improved temporary directory handling
- Added the --basetemp option to pytest commands to specify a base temporary directory, enhancing the management of test artifacts during execution.
- Simplified the upload path for pytest results to focus on the temporary runner directory, ensuring efficient artifact collection.
* Refine pytest upload path in single-postgres.yml for improved artifact collection
- Simplified the upload path for pytest results by removing the specific wildcard in the temporary directory, ensuring more efficient collection of test artifacts during the workflow execution.
* Update workflows to use pipenv-setup@v4.4.0 with editable flag
Upgraded fizyk/actions-reuse pipenv-setup action from v4.2.1 to v4.4.0 which includes built-in support for editable package installation via the editable flag. This simplifies the workflows by removing the explicit pip install -e . steps.
Changes:
- Updated pipenv-setup to v4.4.0 in all workflow files
- Added editable: true parameter to pipenv-setup steps
- Removed separate "Install package in editable mode" steps from dockerised-postgres.yml, single-postgres.yml, and oldest-postgres.yml
This aligns with the upstream pattern established in pytest-mongo and reduces workflow complexity while maintaining the same functionality.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Update workflow files to use pipenv-run@v4.2.1 and refine conditional checks
- Upgraded the pipenv-run action to v4.2.1 in dockerised-postgres.yml, single-postgres.yml, and oldest-postgres.yml for consistency across workflows.
- Refined the conditional check for installing libpq to use matrix.python-version instead of inputs.python-versions in oldest-postgres.yml and single-postgres.yml, improving clarity and accuracy.
These changes enhance the maintainability and consistency of the workflow configurations.
* Refactor socket directory handling in test_executor.py and clean up test_postgres_options_plugin.py
- Updated the socket directory creation in test_executor.py to use basetemp for improved path management, addressing Unix domain socket length limitations.
- Removed unnecessary conftest configuration in test_postgres_options_plugin.py to streamline the pytest setup.
These changes enhance the clarity and efficiency of the test configurations.
* Fix formatting issues in PostgreSQL command templates and update test assertions
- Removed unnecessary spaces in the command templates for both Unix and Windows to ensure proper execution of PostgreSQL commands.
- Updated test assertions in test_executor.py and test_windows_compatibility.py to check for the correct formatting of the `unix_socket_directories` parameter, ensuring consistency across platforms.
These changes enhance the reliability of command execution and improve test accuracy.
* Update test assertions for PostgreSQL command formatting in test_executor.py
- Adjusted assertions in test_actual_postgresql_start_unix and test_actual_postgresql_start_darwin to check for correct formatting of the `unix_socket_directories` parameter, ensuring consistency in command output.
These changes improve the accuracy of the tests related to PostgreSQL command execution.
* Add FreeBSD to platform parameterization in test_executor.py
- Updated the platform_name parameter in test_executor.py to include "FreeBSD", expanding the test coverage for platform-specific functionality.
This change enhances the robustness of the tests by ensuring compatibility with FreeBSD.
* Fixed trailing whitespace in single-postgres.yml and test_executor.py
* Refine pytest temporary directory handling in single-postgres.yml
- Updated the --basetemp option in pytest commands to specify a consistent temporary directory for test artifacts, improving organization and clarity in the workflow.
- Adjusted the artifact upload path to reflect the new temporary directory structure, ensuring efficient collection of test results.
These changes enhance the maintainability and reliability of the testing workflow.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Update test documentation for Windows UNC paths in test_windows_compatibility.py
- Changed the docstring to use a raw string literal for better handling of backslashes in UNC path examples.
- Clarified the description of UNC paths to ensure accurate representation of their format.
These updates improve the clarity and correctness of the test documentation.
* Remove editable package changes extracted to separate PR
The pipenv-setup@v4.4.0 upgrade and editable: true flag additions have
been extracted into a dedicated PR (#1294). Remove those changes from
this branch to avoid duplication and conflicts once that PR is merged.
Made-with: Cursor
* Refactor command quoting in PostgreSQLExecutor for consistency
- Updated the command templates in `PostgreSQLExecutor` to ensure the executable path is always double-quoted, improving compatibility with paths containing spaces.
- Modified tests in `test_windows_compatibility.py` to reflect the changes in command formatting, ensuring that single-quoted PostgreSQL options are preserved correctly within double-quoted command strings.
- Added new tests to verify that the `running()` method properly quotes the executable and datadir, enhancing robustness against shell parsing issues.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Enhance PostgreSQLExecutor to escape apostrophes in unixsocketdir
- Updated the command generation in `PostgreSQLExecutor` to escape single quotes in the `unixsocketdir` parameter, ensuring valid syntax for PostgreSQL GUC strings.
- Refactored the `running()` method to use `subprocess.run` instead of `subprocess.getstatusoutput`, improving compatibility and security by avoiding shell parsing.
- Added regression tests in `test_windows_compatibility.py` to verify correct escaping of apostrophes in `unixsocketdir` and ensure robust command generation.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fix CI for Windows support and align with main workflow conventions
- tests/test_windows_compatibility.py: mark the docstring with a backslash
path example as a raw string so ruff D301 passes on pre-commit.ci.
- .github/workflows/tests.yml: drop windows_postgres_18 and
windows_postgres_16 jobs. ankane/setup-postgres@v1 only ships
PostgreSQL 17 on the Windows runner, so those versions cannot install
and the jobs are guaranteed to fail. Keep windows_postgres_17 and add
the codecov_token secret so coverage uploads work.
- .github/workflows/dockerised-postgres.yml: remove the stale
editable: true flag. Per fizyk in #1294, the editable self-install is
handled by the pytest-postgresql entry in the Pipfile, and the other
workflows on main do not set the flag either.
Made-with: Cursor
* Action fizyk review requests: split Windows CI and skip non-Windows tests
- oldest-postgres.yml: drop redundant runner.os condition from install libpq step
- Extract Windows CI into single-postgres-windows.yml for cleaner maintenance
- single-postgres.yml now serves Linux/macOS only with no platform conditionals
- tests.yml: wire windows_postgres_17 to single-postgres-windows.yml
- test_windows_compatibility.py: add module-level pytestmark to skip on non-Windows
Made-with: Cursor
* Fix three Windows CI failures
- test_executor_init_bad_tmp_path: add parents=True to mkdir so nested
path creation works on Windows (backslash in path literal is a
separator, creating two levels that need parents=True)
- executor.py stop(): set self._process = None after Windows terminate
so mirakuru state is clean and stopped() context manager can call
start() again without timing out
- test_postgres_loader_in_ini: convert backslashes to forward slashes
before writing Windows path into pytest.ini to prevent configparser
from mangling the value as escape sequences
Made-with: Cursor
* Fix Windows restart timeout and macOS xdist race in executor tests
- Override check_subprocess() in PostgreSQLExecutor to use pg_ctl status
instead of checking whether the launcher subprocess (pg_ctl start -w)
is still alive. pg_ctl start -w exits as soon as the server is ready,
so mirakuru's default polling loop always timed out when stopped()
tried to restart the server on Windows. Using pg_ctl status makes
the start/stopped() cycle reliable on all platforms.
- Wrap the psycopg.connect() call in assert_executor_start_stop() with
retry() so that transient 'the database system is starting up' errors
are tolerated. Under parallel xdist runs multiple PostgreSQL instances
start simultaneously; the TCP port opens before recovery completes,
causing an immediate connect to fail even though the server is running.
Made-with: Cursor
* Refactor executor test assertions for path quoting
- Updated assertions in `test_executor_with_special_chars_in_all_paths` to ensure that both `datadir` and `logfile_path` are properly quoted in the command string. This change enhances the reliability of the tests by explicitly checking for the presence of quoted paths, improving compatibility with special characters.
Made-with: Cursor
* Fix connection leak, tighten socket-dir assertion, fix Windows start
- assert_executor_start_stop: capture conn returned by retry() and call
conn.close() so the readiness backend session is released promptly
instead of waiting for GC.
- test_executor_with_special_chars_in_all_paths: strengthen the Unix
unix_socket_directories assertion to include the actual socket_dir
path value, so a missing or mis-quoted path fails the test.
- PostgreSQLExecutor.start(): on Windows, run pg_ctl start -w
synchronously via shell=True rather than delegating to mirakuru's
subprocess polling. pg_ctl exits as soon as the server is ready, so
mirakuru's check_subprocess loop always sees a dead launcher process
and times out; bypassing it removes the 60-second timeout failure
seen in test_noproc_cached_version and related Windows CI jobs.
Made-with: Cursor
* Refine Windows start() and check_subprocess() in executor
- start() Windows branch: replace shell=True/self.command with an argv
list built from the same pieces used in WINDOWS_PROC_START_COMMAND
(executable, datadir, port, logfile, startparams, postgres_options).
Pass env=self.envvars so locale variables reach pg_ctl. Mirrors the
list-form invocation already used in stop().
- check_subprocess(): only return self.running() on Windows (where start()
is synchronous and mirakuru's loop is bypassed). On all other platforms
delegate to super().check_subprocess() so TCPExecutor's port-reachability
check and subprocess-alive check remain in effect, enabling fast
ProcessFinishedWithError detection when pg_ctl fails to start.
- Add missing shlex import.
Made-with: Cursor
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Honor executor timeout in Windows start() path
subprocess.run() on the Windows synchronous start path had no timeout,
so a stuck pg_ctl could block indefinitely. Pass self._timeout (the
configured mirakuru timeout in seconds) to subprocess.run(..., timeout=).
Catch subprocess.TimeoutExpired — subprocess.run already kills the
process before re-raising, so no extra cleanup is needed — and translate
it to mirakuru.exceptions.TimeoutExpired so callers see the same
exception type produced by the non-Windows path.
Also imports TimeoutExpired from mirakuru.exceptions alongside the
existing ProcessFinishedWithError import.
Made-with: Cursor
* Fix output leakage, cross-platform test coverage, and Windows start env/posix
Bug fixes
- executor.py running(): add capture_output=True to subprocess.run so
pg_ctl status output no longer leaks to the terminal on every call;
previously the change from getstatusoutput() dropped the implicit
capture.
- test_windows_compatibility.py: remove the module-level pytestmark
that skipped every test on non-Windows. All tests in the file use
patch() to mock platform.system() and subprocess calls, so none of
them require a real Windows host; the blanket skip was silently hiding
the entire template and process-management test suite from Linux/macOS
CI runs. The unused sys import is removed along with the mark.
Inline comment fixes
- oldest-postgres.yml: narrow the 'install libpq' step condition to
also require runner.os == 'Linux' so 'sudo apt install' is not
attempted on non-Linux runners when the matrix entry contains PyPy.
- executor.py Windows start() branch: build merged_env from
os.environ.copy() updated with self.envvars so system-level PATH and
other vital variables are inherited rather than replaced by the
locale-only dict.
- executor.py Windows start() branch: pass posix=False to
shlex.split(self.startparams) so Windows backslashes in startparams
are treated as literals rather than POSIX escape characters.
Nitpick
- newsfragments/1182.feature.rst: rewrite as a concise user-facing
summary without internal symbol names.
Made-with: Cursor
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* 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.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Update shlex.split for POSIX compatibility and reset process attribute in PostgreSQLExecutor
- Changed shlex.split(self.startparams) to use posix=True for proper handling of backslashes in start parameters on Windows.
- Set self._process to None after terminating the process in the Windows branch to ensure clean state management.
* Update artifact naming in GitHub Actions and adjust shlex.split for Windows compatibility
- Changed the artifact name in the GitHub Actions workflow for PostgreSQL on Windows to include 'windows' for clarity.
- Updated shlex.split in PostgreSQLExecutor to use posix=False, ensuring proper handling of backslashes in start parameters on Windows.
* Enhance PostgreSQLExecutor with wait_for_postgres and timeout handling
- Added wait_for_postgres() call after pg_ctl start to ensure the server is ready for connections.
- Implemented timeout handling in the stop() method to raise TimeoutExpired if the stop command exceeds the configured timeout.
* Update Windows compatibility test to include timeout in pg_ctl stop assertion
- Added timeout parameter to the mock_subprocess call in the TestWindowsCompatibility class to ensure that the timeout handling is properly tested in the pg_ctl stop command.
* Update Windows compatibility test to assert pg_ctl stop command with timeout
- Modified the mock_subprocess assertion in the TestWindowsCompatibility class to include the expected timeout parameter for the pg_ctl stop command, ensuring proper timeout handling is tested.
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>1 parent df79bb9 commit d912c6a
9 files changed
Lines changed: 1422 additions & 33 deletions
File tree
- .github/workflows
- newsfragments
- pytest_postgresql
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
| 55 | + | |
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
46 | 65 | | |
47 | 66 | | |
48 | 67 | | |
| |||
51 | 70 | | |
52 | 71 | | |
53 | 72 | | |
54 | | - | |
| 73 | + | |
55 | 74 | | |
56 | 75 | | |
57 | 76 | | |
58 | 77 | | |
59 | | - | |
| 78 | + | |
60 | 79 | | |
61 | 80 | | |
62 | 81 | | |
63 | | - | |
| 82 | + | |
64 | 83 | | |
65 | 84 | | |
66 | 85 | | |
67 | 86 | | |
68 | | - | |
| 87 | + | |
69 | 88 | | |
70 | 89 | | |
71 | 90 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
80 | 88 | | |
81 | 89 | | |
82 | 90 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
0 commit comments