fix(bitcoin): repair bitcoin-wallet-snap integration tests - #371
Merged
Merged
Conversation
Repair the bitcoin-wallet-snap integration test suite, which could not
run at all from a fresh checkout.
- Point `run-integration.sh` at `jest.integration.config.mjs`. The
script referenced `jest.integration.config.js`, which does not exist,
so Jest failed to resolve a root directory before any test could run.
- Add a `ts-jest` transform to `jest.integration.config.mjs`. Without an
explicit transform, the `@metamask/snaps-jest` preset falls back to
`babel-jest`, which requires `@babel/preset-env` (not installed), so
every test file failed to compile.
- Update `keyring-request.test.ts` to match the current keyring request
response shape. Responses are no longer wrapped in the legacy
`{ pending, result }` envelope, so 11 `toRespondWith` assertions were
changed to expect the raw payload and 2 response destructures were
unwrapped.
Verified with `yarn workspace @metamask/bitcoin-wallet-snap run
test:integration`: 4 suites passed, 53 tests passed, 7 skipped.
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Correct the SignPsbt response assertion type.
Review effort: Lite
Findings: None
What changed in this PR
Repairs the Bitcoin wallet Snap integration-test harness and updates keyring assertions.
Changes:
- Corrects the Jest config path.
- Adds
ts-jesttransformation. - Updates assertions for direct keyring responses.
- A
SignPsbtassertion still uses the inaccurateFillPsbtResponsetype.
| File | Summary |
|---|---|
packages/bitcoin-wallet-snap/jest.integration.config.mjs |
Adds TypeScript transformation. |
packages/bitcoin-wallet-snap/integration-test/run-integration.sh |
Uses the correct Jest config path. |
packages/bitcoin-wallet-snap/integration-test/keyring-request.test.ts |
Updates response assertions and unwrapping. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The `broadcasts a PSBT successfully` test issues `AccountCapability.SignPsbt` but asserted its response as `FillPsbtResponse`, which only declares `psbt`. The handler returns `SignPsbtResponse` (`psbt`, `txid`, optional `canBeMalleable`), so the assertion named a type from a different capability. Because this is a type assertion, TypeScript never verified it: the cast compiles regardless, and the tests pass either way. Swapping it to `SignPsbtResponse` restores the type accuracy that the assertion was meant to provide. Verified with `yarn workspace @metamask/bitcoin-wallet-snap run test:integration` (4 suites passed, 53 tests passed, 7 skipped) and `tsc --noEmit`.
Contributor
Author
Fixed in commit fda2a4d |
|
Julink-eth
approved these changes
Sep 25, 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
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.
Explanation
The
bitcoin-wallet-snapintegration suite could not run at all from a fresh checkout — it failed before executing a single test. Three separate defects were stacked on top of each other:Wrong Jest config path.
integration-test/run-integration.shinvokedjest --config jest.integration.config.js, but the file on disk isjest.integration.config.mjs. Jest aborted withCan't find a root directory while resolving a config file pathand never discovered the test files.No transform configured. With the path corrected, the
@metamask/snaps-jestpreset fell back tobabel-jest, which resolves@babel/preset-env— a package this repo does not depend on. Every test file failed to compile. This is why an explicittransformis required rather than optional.Stale keyring response assertions.
keyring-request.test.tsstill asserted the legacy v1 keyring envelope,{ pending: false, result: <payload> }. Keyring requests now return the payload directly, so 11toRespondWithassertions failed on shape and 2 response destructures double-unwrappedresult.result.The fixes are correspondingly narrow: correct the config filename, add
transform: { '^.+\\.(t|j)sx?$': 'ts-jest' }to the config, and update the assertions to the current response shape.Nothing here changes Snap behavior — it is test-harness only, which is why it does not need to touch another package.
References
fix/bitcoin-snap-integration-tests(based onmain)yarn workspace @metamask/bitcoin-wallet-snap run test:integrationChecklist