refactor(db): introduce DatabaseContext pattern for injectable db in tests#1013
refactor(db): introduce DatabaseContext pattern for injectable db in tests#1013
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — Clean refactoring that introduces a DatabaseContext class wrapping the Drizzle db and pg pool, replacing module-level singletons with an injectable pattern. Backward compatibility is fully preserved: getDb(), closeDb(), and _setTestDb() all continue to work without consumer changes. CI is green across all 7 checks.
Notes
Minor comment inaccuracy (not blocking): The comment in closeDb() at src/db/client.ts:106-107 says "Only close if it's a real DatabaseContext / Skip if it was set via _setTestDb" — but the code doesn't skip anything. It unconditionally calls _defaultContext.close() and relies on the try/catch to handle mock contexts (whose close() is a no-op). The comment implies conditional behavior that doesn't exist. Consider simplifying to:
// close() is safe for both real pools and mock contexts (which have a no-op close)
Subtle behavior change (verified safe): The old closeDb() only reset pool/db but did NOT clear _testDbOverride. Now closeDb() nulls _defaultContext, which also clears any test override. This is safe because: (1) integration tests call _setTestDb(null) before closeDb(), and (2) unit tests call resetDbState() which does both. All 6596 tests pass, confirming no breakage.
🕵️ claude-code · claude-opus-4-6 · run details
Summary
DatabaseContextclass with acreateDatabaseContext()factory that encapsulates the Drizzle db and pg pool, replacing the module-levellet db/let poolsingletons insrc/db/client.tssetDefaultDatabaseContext()as the proper DI mechanism for tests, replacing the_setTestDb()hack (kept as a deprecated backward-compat wrapper)getDb(),closeDb(),_setTestDb()— zero consumer changes neededDrizzleDbtype for use in typed DI scenariostests/helpers/sharedMocks.tsto exposesetDefaultDatabaseContextand_setTestDbinmockDbClientModuletests/unit/db/client.test.tswith full coverage of the newDatabaseContext,createDatabaseContext(), andsetDefaultDatabaseContext()APIsCard: https://trello.com/c/VAKnTLgw/531-as-a-developer-i-want-db-clientts-refactored-into-a-databasecontext-pattern-so-that-repositories-are-unit-testable-without-a-rea
Test plan
npm testpasses — 6596 tests across 344 test files, all greennpm run typecheckpasses — zero TypeScript errorsbiome checkpasses — zero lint errorstests/unit/db/client.test.ts— 21 tests including new coverage forsetDefaultDatabaseContext,createDatabaseContext, andDatabaseContextclassgetDb()mock invi.mock('../../src/db/client.js')still works)🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details