Repository navigation
Fix race conditions when running tests in separate worktrees - #433
Open
Thom Chiovoloni (thomcc-work) wants to merge 4 commits into
Open
Thom Chiovoloni (thomcc-work) wants to merge 4 commits into
Thom Chiovoloni (thomcc-work) wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The test fixture retains symlinked temporary paths, causing the working-directory assertion to fail against the physical path recorded by os.getcwd() even when locking is correct.
1 open issue
What changed in this PR
This PR adds cooperative locking to prevent pg_durable test runs in separate worktrees from colliding over a shared PostgreSQL installation.
Changes:
- Adds a shared lock wrapper and integrates it with local test runners and PostgreSQL management commands.
- Adds coordination tests and CI checks, and stops startup when extension installation fails.
- Documents queued runs, manual command wrapping, and exclusive debugging sessions.
| File | Description |
|---|---|
| tests/test_pg_lock.py | Tests locking, nested runners, signals, and entry points. |
| scripts/with-pg-lock.py | Implements the shared PostgreSQL lock wrapper. |
| scripts/test-upgrade.sh | Locks upgrade runs and warns about unreserved servers. |
| scripts/test-unit.sh | Locks unit test execution. |
| scripts/test-shutdown.sh | Locks shutdown tests. |
| scripts/test-epoch-race.sh | Locks epoch race tests. |
| scripts/test-e2e-local.sh | Locks local E2E runs and warns about unreserved servers. |
| scripts/test-coverage.sh | Locks coverage runs. |
| scripts/test-all-local.sh | Locks aggregate test runs. |
| scripts/pg-stop.sh | Locks local PostgreSQL shutdowns. |
| scripts/pg-start.sh | Locks startup and propagates installation failures. |
| scripts/pg-reset.sh | Locks database resets. |
| scripts/pg-common.sh | Adds shared locking helpers and signal traps. |
| README.md | Introduces cross-worktree coordination guidance. |
| Makefile | Wraps local targets and locks the full regression sequence. |
| docs/TESTING.md | Documents locking behavior and reserved debugging sessions. |
| docs/E2E_TESTING.md | Clarifies debugging risks after --keep. |
| CONTRIBUTING.md | Adds concurrent-worktree testing guidance. |
| .github/workflows/ci.yml | Adds locking checks and uses the unit test runner. |
| .github/copilot-instructions.md | Directs agents to use coordinated test commands. |
Review effort: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
SIGTERM can release the lock while a wrapped shell’s foreground child continues running, allowing queued commands to overlap; a supervisor must retain lock ownership until foreground work stops.
1 open issue
1 resolved since last review
Review effort: Balanced
| def exec_command(command): | ||
| os.environ["_PG_DURABLE_LOCK_EXEC_PID"] = str(os.getpid()) | ||
| try: | ||
| os.execvp(command[0], command) |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes the issue where if you have two worktrees of pg_durable on your machine, and agents working in both of them -- they share a PG installation, and thus collide. (See pgcentralfoundation/pgrx#2376 for the general PG issue).
This fixes it in a... hacky way. To get the 'fix' you have to run things in a script that takes a lock. Easy for an agent to do every time, less easy for a human. That said, the failure mode is the same as the status quo, so it's not the end of the world.