Repository navigation
test: open and close test postgresql pools through a shared helper - #3560
Merged
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
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.
Problem
PostgreSQL tests open a pool and close it in an after hook with the same two lines:
const pool = new pg.Pool({...})and thent.after(() => pool.end()). A survey oftests/on main found 65 such pairs in 26 files.pool.end()resolves once the pool's idle clients are removed, before their sessions close. When the test's database is dropped right after (DROP DATABASE ... WITH (FORCE)), the drop can end a session that is still closing. The server's 57P01 then reaches the pool, which has no error listener, and the test fails in its teardown. That is finding 2379, fixed for the revision fixture in #3547.withPoolintests/helpers/postgres-database.mjsalready avoids it by waiting for each client's own end. The hand-written after hooks do not.Change
tests/helpers/postgres-database.mjs:withPool's close (end the pool, then wait for each client'send) moves into a privatecloser(pool).withPooluses it unchanged.testPool(context, config)returnsnew pg.Pool(config)and registerscloser(pool)withcontext.afterat once, where the old after hook was registered.contextis a test context or anything withafter(the repository credential resource scope).testPool, with the same pool configuration.postgres-presets' two-pool hook (Promise.allof two ends) becomes twotestPoolcalls. Imports ofpgthat became unused are removed.postgres-platform-state-kubernetes.test.mjs, whose four case pools end right before their case database is dropped in the same process (the 2379 shape).native-admin-session-postgres.test.mjs(feat: add opt-in host-only Agent browser sessions #3255 rewrites its imports).pool.end(), or that guard against a second end.Test-only. No product code or assertions change.
Verification
testPoolignoring itsconfig(connecting to a closed port) fails all nine postgres-platform caller files (openshell-git-routes, openshell-standing-git-transfer, prepared-standing-removal, presets, repository-broker-receipts, repository-sessions, repository-transactions, service-account-issuance-recovery, standing-approval-api); the two startup files never connect, so they survive.withPool's existing behaviour, now shared.tsc --build tsconfig.json tests/tsconfig.json, ESLint and Prettier are clean.git merge-tree: feat: assign configured OpenClaw roles to Agent users #906, fix(controller): close refused native-admin sockets before audit settlement #1707, feat: add Gateway managed mode to Agent deployments #2982 and feat: add opt-in host-only Agent browser sessions #3255 (the open PRs on these files) get no new conflicted file or hunk. This PR merges cleanly with test: assert API error responses through a shared helper #3558.Risks
Low. Teardown now also waits until the pool's sessions have closed, as
withPoolalready does. A pool whose client never closes would hold its after hook, butpool.end()already waits for checked-out clients before it resolves.