test(keymaster): cover WalletSQLite defaults and connection guards - #832
Merged
Conversation
db/sqlite.ts reaches 100% on both branches (53.8% -> 100%) and lines
(93.1% -> 100%). Four of the six untaken branches were unexercised
default parameters on the constructor and create(); the other two were
the defensive `!this.db` guards, reachable only by stubbing connect() to
a no-op, since a successful connect always sets the handle.
Exercising the default constructor arguments surfaced a real asymmetry
between the two wallet backends:
// WalletJson.saveWallet
if (!fs.existsSync(this.dataFolder)) {
fs.mkdirSync(this.dataFolder, { recursive: true });
}
// WalletSQLite.connect — no equivalent
WalletSQLite opens `${dataFolder}/${file}` directly, so an absent
directory fails with a raw SQLITE_CANTOPEN rather than being created or
reported clearly. The existing tests never hit it because they always
pass an already-created temp dir; only the default-argument path exposes
it.
Pinned by test asserting current behaviour rather than changed — whether
the two implementations of the same interface should agree is a design
call. Worth knowing that a fresh deployment relying on the sqlite wallet
needs `data/` to exist beforehand.
Repo branches 86.46% -> 86.68%.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
macterra
enabled auto-merge (squash)
August 3, 2026 00:06
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.
Result
db/sqlite.tsbranchesdb/sqlite.tslines4 tests. 2,066 passing, eslint clean. Test-only.
A real asymmetry, surfaced by covering the default arguments
The two wallet backends implement the same interface but differ on folder creation:
WalletSQLiteopens${dataFolder}/${file}directly, so an absent directory fails with a rawSQLITE_CANTOPENrather than being created, or reported with a message that says what is wrong.The existing tests never hit this because they always pass an already-created temp dir. Only the default-argument path exposes it — which is precisely the branch that was uncovered, so the gap in coverage and the gap in behaviour were the same gap.
Pinned by test asserting current behaviour rather than changed. Whether two implementations of one interface should agree here is a design call, not something to decide inside a coverage PR. Practically: a fresh deployment using the sqlite wallet needs
data/to exist beforehand. The Docker images likely provide it via a volume mount, so this may never have bitten anyone.The other branches
Four unexercised default parameters (
walletFileNameanddataFolder, on both the constructor andcreate()), and the two defensive!this.dbguards — reachable only by stubbingconnect()to a no-op, since a successful connect always sets the handle. That is noted in a comment so the next reader knows the guard is otherwise dead code.🤖 Generated with Claude Code