fix(test): remove app.Before() calls that overwrite test DB config with pgx driver#21975
Open
fix(test): remove app.Before() calls that overwrite test DB config with pgx driver#21975
Conversation
Contributor
|
I see you updated files related to
|
Contributor
|
✅ No conflicts with other open PRs targeting |
Fletch153
force-pushed
the
fix/CORE-2388-db-connection-leakage
branch
from
3879497 to
a02ad86
Compare
Fletch153
changed the title
Fletch153
force-pushed
the
fix/CORE-2388-db-connection-leakage
branch
from
b1c30ad to
38c8d97
Compare
TestShell_BeforeNode and TestShell_RunNode_WithBeforeNode were calling app.Before(c) after pre-setting shell.Config with a txdb-wrapped config via configtest.NewGeneralConfig. app.Before unconditionally ran opts.New() → CoreDefaults() which set Database.DriverName back to pgx and assigned the fresh config to s.Config, silently overwriting the test's txdb config. BeforeNode → pg.NewLockedDB then opened real pgx connections against the shared chainlink_test database, persisting keystore state and leaking it to other tests (flaking Test_CSAKeyStore_E2E) and eating slots from the server-wide max_connections budget (causing the mass 25-minute chan-receive timeouts in CORE-2388). - core/cmd/app.go: guard opts.New() so a pre-set s.Config is preserved. Defense-in-depth against any future test hitting the same trap. - core/cmd/shell_local.go: nil-guard CloseLogger in afterNode (parity with app.After; tests calling BeforeNode directly never set it). - core/cmd/shell_local_test.go: remove the unnecessary app.Before(c) calls. Drop the incorrect_password subtests — they were load-bearing on the leak; the underlying keystore invariant is covered by TestMasterKeystore_Unlock_Save in core/services/keystore. Refs: CORE-2388, CORE-2370 Supersedes: #21504
Fletch153
force-pushed
the
fix/CORE-2388-db-connection-leakage
branch
from
38c8d97 to
69a7b6a
Compare
|
mchain0
approved these changes
Apr 13, 2026
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
2 participants
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.
Summary
TestShell_BeforeNodeandTestShell_RunNode_WithBeforeNodecalledapp.Before(c)beforeBeforeNode(c). TheBeforehook unconditionally creates a fresh config with the defaultpgxdriver (viaCoreDefaults()), overwriting the test'stxdb-wrapped config set byconfigtest.NewGeneralConfig.This caused
BeforeNode→pg.NewLockedDB→openDBto usecfg.DriverName()=pgxinstead oftxdb, opening real DB connections that leaked state between tests and exhausted the connection pool.Root Cause Trace
configtest.NewGeneralConfig→Database.DriverName = DriverTxWrappedPostgresshell.Config = cfgapp.Before(c)→opts.New()→CoreDefaults()→Database.DriverName = DriverPostgresBeforesetss.Config = cfg→ overwrites txdb with pgxBeforeNode→pg.NewLockedDB(cfg.Database())→openDB→NewConnection(uri, cfg.DriverName())→ usespgxFix
Remove the
app.Before(c)calls.BeforeNodeonly needsConfigandLogger— both already set by the test. Other tests in the same file (lines 258, 342, 420) correctly callBeforeNodedirectly withoutapp.Before().Test-only change. No production code modified.
Related
Fixes: CORE-2388