feat(e2e): new offer sync validation in buy BTC test #17430
Conversation
minor name refactoring
WalkthroughThis pull request adds a new JSON file that provides updated Bitcoin quote data with detailed information for cryptocurrency transactions. The new data includes exchange details, fiat and cryptocurrency currencies, rates, payment methods, and various limits and identifiers. Additionally, multiple test files and a page object file have been updated to integrate the new JSON data. Method names within the trading page object have been refactored—from names like ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 0
🧹 Nitpick comments (1)
packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts (1)
408-422: Enhanced reusability by parameterizing the quotes validation.The
validateBuyQuotesmethod now accepts aquotesResponseparameter instead of hardcoding a reference tobuyQuotesBTC. This makes the method more flexible and reusable across different test scenarios, including the new offer refresh validation.Consider adding type annotations to the
quotesResponseparameter to make the expected data structure clearer:- async validateBuyQuotes(quotesResponse: any[]) { + async validateBuyQuotes(quotesResponse: Array<{ + paymentMethod: string; + error?: string; + exchange: string; + receiveStringAmount: string; + }>) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/suite-desktop-core/e2e/fixtures/invity/buy/quotes-bitcoin-update.json(1 hunks)packages/suite-desktop-core/e2e/fixtures/invity/index.ts(2 hunks)packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts(5 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts(3 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-ethereum.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/sell-bitcoin.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/sell-ethereum-token.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/sell-solana.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/suite-desktop-core/e2e/fixtures/invity/buy/quotes-bitcoin-update.json
🧰 Additional context used
🧠 Learnings (4)
packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts (2)
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17199
File: packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts:34-38
Timestamp: 2025-02-24T15:31:48.018Z
Learning: Network configuration changes (enableNetwork/disableNetwork) in e2e tests only affect the specific suite instance and modify network visibility within that suite. Test isolation is handled by the framework.
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17423
File: packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts:38-39
Timestamp: 2025-03-05T16:14:27.095Z
Learning: In the Trezor Suite e2e tests, Bitcoin is discovered by default, so there's no need to call `await dashboardPage.discoveryShouldFinish()` after activating coins in tests that only work with Bitcoin.
packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts (2)
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17199
File: packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts:34-38
Timestamp: 2025-02-24T15:31:48.018Z
Learning: Network configuration changes (enableNetwork/disableNetwork) in e2e tests only affect the specific suite instance and modify network visibility within that suite. Test isolation is handled by the framework.
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17423
File: packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts:38-39
Timestamp: 2025-03-05T16:14:27.095Z
Learning: In the Trezor Suite e2e tests, Bitcoin is discovered by default, so there's no need to call `await dashboardPage.discoveryShouldFinish()` after activating coins in tests that only work with Bitcoin.
packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts (1)
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17199
File: packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts:34-38
Timestamp: 2025-02-24T15:31:48.018Z
Learning: Network configuration changes (enableNetwork/disableNetwork) in e2e tests only affect the specific suite instance and modify network visibility within that suite. Test isolation is handled by the framework.
packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts (2)
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17423
File: packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts:38-39
Timestamp: 2025-03-05T16:14:27.095Z
Learning: In the Trezor Suite e2e tests, Bitcoin is discovered by default, so there's no need to call `await dashboardPage.discoveryShouldFinish()` after activating coins in tests that only work with Bitcoin.
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17199
File: packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts:34-38
Timestamp: 2025-02-24T15:31:48.018Z
Learning: Network configuration changes (enableNetwork/disableNetwork) in e2e tests only affect the specific suite instance and modify network visibility within that suite. Test isolation is handled by the framework.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: build-web
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (24)
packages/suite-desktop-core/e2e/fixtures/invity/index.ts (2)
2-2: New fixture file added for updated Bitcoin quotes.Good addition of a new import for
buyQuotesBTCUpdatefrom 'quotes-bitcoin-update.json'. This aligns with the PR objective of enhancing offer sync validation in buy BTC tests.
90-90: Appropriate export of the new fixture.Correctly exporting the newly imported
buyQuotesBTCUpdatefixture to make it available for test files.packages/suite-desktop-core/e2e/tests/trading/sell-ethereum-token.test.ts (1)
51-51: Method name refactoring improves clarity.Good renaming from
setYouSellAmounttofillSellFormwhich is more descriptive of the action being performed. This is a positive change for code readability and maintainability.packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts (1)
55-55: Clear method renaming enhances test readability.Good refactoring from
setYouSwapAmounttofillSwapFormwhich provides a more intuitive description of the action being performed.Based on the retrieved learnings, I noticed that you've properly implemented network configuration in your e2e tests, understanding that these changes only affect the specific suite instance under test.
packages/suite-desktop-core/e2e/tests/trading/buy-ethereum.test.ts (1)
37-37: Consistent method renaming improves API clarity.Good renaming from
setYouBuyAmounttofillBuyFormwhich maintains consistency with the other trading form methods. This refactoring provides a more intuitive understanding of what the method does.packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts (1)
46-46: Method name refactoring is consistent with project changesThe method name has been updated from
setYouSwapAmounttofillSwapForm, which is part of a broader refactoring across the trading page object methods. This change improves readability by making the method name more descriptive of its actual purpose - filling out a form rather than just setting an amount.packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts (1)
46-46: Method name change maintains consistencyThis change from
setYouSwapAmounttofillSwapFormaligns with the method naming pattern established across other trading test files. The functionality remains the same while the name more accurately describes the action being performed.packages/suite-desktop-core/e2e/tests/trading/sell-solana.test.ts (1)
67-67: Method renamed for consistent APIThe method name change from
setYouSellAmounttofillSellFormfollows the same pattern applied to other trading methods. This makes the trading page API more consistent (fillBuyForm, fillSellForm, fillSwapForm) and better describes that the method completes the entire form rather than just setting an amount.packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts (2)
46-46: Method name refactoring improves code clarityThe changed method name from
setYouSwapAmounttofillSwapFormis more descriptive and follows the consistent naming pattern established across all trading functionality. This naming convention better indicates that the method handles the complete form filling process.
36-38: Leveraging network configuration knowledge effectivelyYour approach to explicitly enabling/disabling required networks (enabling SOL and ETH while disabling BTC) is correctly implemented. This aligns with previous learnings that network configuration changes only affect the specific suite instance and that test isolation is handled by the framework.
packages/suite-desktop-core/e2e/tests/trading/sell-bitcoin.test.ts (1)
49-49: Method name change looks good and is more descriptive.The renamed method
fillSellFormis more descriptive than the previoussetYouSellAmount, as it better represents the action of populating the entire sell form rather than just setting an amount. This change improves code clarity and is consistent with similar method renames across the codebase.packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.test.ts (1)
47-51: Method name change is consistent and more descriptive.The renamed method
fillBuyFormbetter represents the action of filling the entire form rather than just setting an amount. This change maintains naming consistency with other similar methods throughout the trading test suite.packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts (8)
4-10: Good addition of the new import for updated quote data.The addition of
buyQuotesBTCUpdateenables testing of the offer refresh functionality, which is an important part of the test case for validating sync behavior.
18-25: Clear constant definitions for the updated test flow.These new constants properly define expected values for the updated flow. The variable names are descriptive and follow the established naming pattern. The comment on line 21 helps explain why index 11 is chosen for the second offer.
38-44: Test title and implementation now match the PR objective.The test has been updated to focus on "Buy Bitcoin from compared offer" which aligns with the PR objective of testing offer sync validation. The first step correctly uses the renamed
fillBuyFormmethod.
46-52: Good addition of form validation check.Adding the check for the formatted fiat input value (line 47) ensures the form is correctly displaying the expected value after filling. The validation of quotes using the parameterized method is a good approach.
54-62: Excellent implementation of offer refresh validation.This new test step properly tests the offer refresh functionality triggered by changing the fiat input. It:
- Sets up a route mock for updated quotes
- Waits for the quote request to be sent
- Updates the input value
- Waits for the request to complete
- Validates the updated quotes are displayed correctly
This implementation addresses the PR objective of testing offer sync validation without relying on
page.clock.forward.
64-66: Concise step for selecting the second offer.This step clearly demonstrates selecting the second offer from the comparison view, which is an essential part of the test flow.
71-71: Correctly updated expected confirmation value.The test now correctly expects the updated fiat amount in the confirmation screen, which matches the selected second offer.
81-81: Method name change is consistent with other updates.The renamed method
fillBuyFormis used consistently in both test cases, improving clarity and maintainability.packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts (4)
7-7: Imported invityEndpoint directly instead of through a relative path.This change simplifies the import and helps maintain consistency in importing the endpoint definitions.
273-297: Method renamed for clarity and improved implementation.The method renaming from
setYouBuyAmounttofillBuyFormbetter represents the function's purpose - it fills out the entire buy form, not just setting an amount. The implementation remains solid, with proper awaiting on network requests and validations.
300-323: Method renamed for clarity and implementation remains solid.The method renaming from
setYouSellAmounttofillSellFormfollows the same pattern as the buy form method. The implementation includes proper error checking for insufficient funds, which is a good practice.
326-352: Method renamed to be consistent with other form-filling methods.The method renaming from
setYouSwapAmounttofillSwapFormmaintains consistency with the other method renames. The implementation properly waits for account changes to take effect before proceeding.
minor name refactoring
Description
This starts covering some of the advance trading scenarios. I wanted originally just move time in compared offer view. But it seems the refresh is not tied to browser time. So page.clock.forward did nothing. Instead I used change of fiat input to trigger one.
Related Issue
Resolve 17364
Screenshots: