fix: add ledger test for bitcoin api calls #5581
Conversation
WalkthroughThe recent update involves incrementing the version of the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (2)
- package.json (1 hunks)
- tests/specs/ledger/ledger.spec.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- package.json
Additional comments not posted (3)
tests/specs/ledger/ledger.spec.ts (3)
2-2: Type import for HomePage added.This import is correctly added as a type import, which ensures it's used for type checking and doesn't result in runtime overhead.
4-4: Import of SettingsSelectors added.This import is necessary for accessing selectors used in the test cases, ensuring that the tests are robust and maintainable.
14-19: New functioninterceptBitcoinRequestsdefined.The function is well-implemented for intercepting specific API requests. However, consider verifying the appropriateness of the 1000 ms timeout in different network conditions.
| test('there are no bitcoin requests', async ({ homePage }) => { | ||
| const requestPromise = interceptBitcoinRequests(homePage); | ||
|
|
||
| await homePage.page.getByTestId(SettingsSelectors.CurrentAccountDisplayName).click(); | ||
|
|
||
| await test | ||
| .expect(async () => await test.expect(requestPromise).rejects.toThrowError()) | ||
| .toPass() | ||
| .catch(); | ||
| }); |
There was a problem hiding this comment.
Tip
Codebase Verification
Explicit error handling needed in test case.
The test case for ensuring no Bitcoin API calls is correctly structured. However, the catch block is currently empty, which may suppress errors silently. Ensure that errors are handled explicitly to catch and address any potential issues during test execution.
- File:
tests/specs/ledger/ledger.spec.ts - Lines: 46-55
Analysis chain
Test case for no Bitcoin API calls.
The test case is correctly structured to ensure that no Bitcoin API calls are made under specific conditions. However, ensure that the catch block is appropriately handling any potential errors that could arise during the test execution.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the error handling in the test case.
# Test: Search for error handling patterns in other tests.
rg --type python $'catch'
Length of output: 55
Script:
#!/bin/bash
# Description: Verify the error handling in the test case.
# Test: Search for error handling patterns in TypeScript tests.
rg --type typescript 'catch'
Length of output: 63
Script:
#!/bin/bash
# Description: Verify the error handling in the test case.
# Test: Search for error handling patterns in TypeScript tests.
rg --type ts 'catch'
Length of output: 6694
fbwoolf
force-pushed
the
fix/ledger-test-for-bitcoin-api-calls
branch
from
e35506e
to
00804b3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (2)
- package.json (1 hunks)
- tests/specs/ledger/ledger.spec.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- package.json
- tests/specs/ledger/ledger.spec.ts
There was a problem hiding this comment.
Great to see test coverage expanded @fbwoolf
So if bitcoin requests are made, the tests here fail?
|
Wrong issue referenced btw 
|
How do I ref issues in the other repo? 🤔 Can't include in commit msg? |
You can do it in the commit message using the full path - |
fbwoolf
force-pushed
the
fix/ledger-test-for-bitcoin-api-calls
branch
from
00804b3
to
cbd52ae
Compare
fbwoolf
force-pushed
the
fix/ledger-test-for-bitcoin-api-calls
branch
from
cbd52ae
to
e18cc9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (2)
- package.json (1 hunks)
- tests/specs/ledger/ledger.spec.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- package.json
- tests/specs/ledger/ledger.spec.ts
Did a bunch of research and testing to understand how to write a test to see if we are making any bitcoin api calls on the homepage if only Stacks keys are present. The test here checks a bug where the requests were being made when a user clicked to change accounts. I tried quite a few things attempting to catch the timeout exception correctly if no requests are made, and I ended up coming across this that helped me: microsoft/playwright#20372 (comment)
Hoping this is right. I made sure the test failed
withBitcoinKeysOnlyandwithStacksAndBitcoinKeys, but that it passeswithStacksKeysOnly...so I think it is working correctly.While working on this, I realized there was a bug with
useInfiniteQuerywhich was causing the linked bug where queries were running even though we were disabling them with!!address, see:https://stackoverflow.com/questions/72753877/useinfinitequerys-enabled-option-in-react-query-doesnt-work
And, fixed/installed here from mono repo: leather-io/mono#246
Summary by CodeRabbit
Chores
@leather.io/querydependency to version0.9.3.Tests