feat(e2e): Expands verification of our Trading tests based on feedback #17363
Conversation
| @@ -1,5 +1,4 @@ | |||
| { | |||
| "status": "SEND_CRYPTO", | |||
| "destinationAddress": "ENk2eeP4umP6cjAGRsVG4NEVKEVQmRn6JEpN8hubv2Hf", | |||
| "destinationPaymentExtraId": "6d666a5f-b99c-4482-b8bc-2df04fc11b7b" | |||
There was a problem hiding this comment.
This value shouldn't be returned for Solana
| @@ -5,7 +5,7 @@ | |||
| "quoteId": "9fd5573d-fb81-423b-84b6-31903770fecf", | |||
| "status": "CONFIRM", | |||
| "statusUrl": "https://changenow.io/exchange/txs/3b45492e83ae8e", | |||
| "exchange": "changenowfr", | |||
| "exchange": "changeherofr", | |||
There was a problem hiding this comment.
to match best quote
WalkthroughThe pull request implements updates across component files, JSON fixtures, and end-to-end tests to improve testability and streamline trade confirmation processes. In UI components, several elements now include 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (20)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (16)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (12)
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
packages/components/src/components/NewModal/NewModal.tsx(1 hunks)packages/suite-desktop-core/e2e/fixtures/invity/sell/watch-solana.json(1 hunks)packages/suite-desktop-core/e2e/fixtures/invity/swap/trade-solana-usdc.json(1 hunks)packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.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(2 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/trading/sell-bitcoin.test.ts(3 hunks)packages/suite-desktop-core/e2e/tests/trading/sell-ethereum-token.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/trading/sell-solana.test.ts(4 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts(5 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts(3 hunks)packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts(3 hunks)packages/suite/src/views/wallet/trading/common/TradingSelectedOffer/TradingInfo/TradingInfoExchangeType.tsx(1 hunks)packages/suite/src/views/wallet/trading/common/TradingTransactionId.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/suite-desktop-core/e2e/fixtures/invity/sell/watch-solana.json
🧰 Additional context used
🧠 Learnings (5)
packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.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-coin-to-token.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/sell-bitcoin.test.ts (1)
Learnt from: Vere-Grey
PR: trezor/trezor-suite#16889
File: packages/suite-desktop-core/e2e/fixtures/invity/sell/trade-bitcoin.json:3-5
Timestamp: 2025-02-07T14:56:05.894Z
Learning: The test suite intentionally uses mainnet addresses (e.g. "bc1" prefixed Bitcoin addresses) in test fixtures while mocking all external service interactions. This approach was chosen over using testnet addresses with live endpoints due to previous timeout issues encountered with Invity's testnet endpoints.
packages/suite-desktop-core/e2e/tests/trading/sell-solana.test.ts (1)
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17018
File: packages/suite-desktop-core/e2e/fixtures/invity/sell/trade-solana.json:6-6
Timestamp: 2025-02-15T07:33:25.449Z
Learning: The "pk_live_" prefixed keys in Moonpay API mocked responses are public keys and don't need to be replaced with placeholder values in test fixtures.
packages/suite-desktop-core/e2e/tests/trading/swap-coins.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.
⏰ 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: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (48)
packages/suite/src/views/wallet/trading/common/TradingTransactionId.tsx (1)
41-41: Added test identifier for transaction ID element.Adding the data-testid attribute improves testability by providing a stable selector for E2E tests to target the transaction ID element. This aligns with the PR objective of expanding verification in Trading tests.
packages/suite/src/views/wallet/trading/common/TradingSelectedOffer/TradingInfo/TradingInfoExchangeType.tsx (1)
25-25: Added test identifier for exchange type information.The data-testid attribute enhances E2E test coverage by providing a reliable selector for verifying the exchange type display. This supports the PR's goal of increasing verification of trading values.
packages/components/src/components/NewModal/NewModal.tsx (2)
124-128: Added test identifier for modal header.Adding a data-testid to the modal header makes it easier to verify the content in E2E tests. This is particularly useful for testing user flows that involve modal interactions and confirms the PR's focus on expanding verification.
129-138: Added test identifier for modal description paragraph.The data-testid for the description paragraph enhances testability by providing a specific selector for verification in E2E tests. This addition seems to support the mentioned "implementation of an ending redirect" in the PR objectives, where confirmation dialogs with descriptive text would need to be verified.
packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts (5)
20-20: Improved test readability by extracting the formatted address.Good change extracting the formatted address into a named constant. This enhances readability and follows the pattern already established for other formatted values.
59-59: Enhanced the confirmTrade method signature.The updated call to
confirmTradenow explicitly requires the account name ('Solana #1') and the formatted address, which is a good improvement that makes the intention clearer and the test more robust.
82-82: Added validation for the device prompt header.Good addition - verifying that the device prompt properly displays the account name ('Solana #1') is important for ensuring the user knows which account is being used for the transaction.
85-88: Enhanced device prompt validation for crypto amounts and fees.The change from
cryptoAmountOftocryptoAmountWithSymbolOfand the addition of a fee validation improves the test by ensuring:
- The displayed amount includes the currency symbol
- The transaction fee is a non-zero value
This provides more comprehensive verification of what the user will see during the transaction flow.
96-103: Added comprehensive confirmation value verification.Good addition of verification steps for the transaction details after approval. This ensures the user is shown the correct:
- Send and receive amounts with proper formatting
- Exchange type information
- Provider details
These checks are crucial for validating the entire transaction experience.
packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts (5)
20-20: Improved test readability with consistent address formatting.Good addition of the formatted address constant, maintaining consistency with other tests and improving code readability.
59-59: Enhanced confirmTrade with explicit account name and address.The updated method signature for
confirmTradenow requires two explicit parameters: the account name ('Bitcoin #1') and the formatted receive address. This makes the test more explicit about the trading destination.
82-82: Added validation for transaction source account.Good addition verifying that the device prompt header correctly displays 'Solana #1' as the source account, which ensures the user is informed about which account is being used for the transaction.
85-88: Enhanced transaction amount and fee verification.Good update to use
cryptoAmountWithSymbolOffor validating the full amount display (including currency symbol) and adding a check that the fee is greater than zero. This ensures proper display of transaction costs to the user.
96-103: Added comprehensive confirmation value verification.Good addition of assertions to verify all key transaction details after approval, including:
- Send and receive amounts with proper formatting
- Exchange type information ('Fixed-rate offer')
- Provider details
This provides validation of the complete transaction confirmation experience.
packages/suite-desktop-core/e2e/tests/trading/sell-ethereum-token.test.ts (2)
21-21: Added commented address formatting for future implementation.The addition of a commented line for formatting the destination address suggests preparation for future implementation. Ensure the import for
formatAddressis also added when uncommenting this code.Is there a specific reason why this part is commented out? When implementing it in the future, remember to add the missing import:
+ import { formatAddress } from '../../support/common';
79-84: Enhanced device prompt validation in commented code.The commented-out code includes improvements to the device prompt validation, following the same pattern as the other test files:
- Verifying the header paragraph contains the account name
- Validating the formatted address display
- Using
cryptoAmountWithSymbolOffor amount verification- Adding a fee verification
This will provide more comprehensive testing once implemented, consistent with the other tests.
packages/suite-desktop-core/e2e/tests/trading/sell-bitcoin.test.ts (3)
11-11: Added import for formatAddress utility.Good addition of the import for the
formatAddressfunction, which is used to format the provider's destination address.
23-23: Added formatted address constant for improved readability.Good extraction of the formatted address into a named constant, which improves code readability and follows the pattern established in other test files.
77-82: Enhanced device prompt validation for comprehensive user experience testing.Excellent improvements to the device prompt verification:
- Validating that the header paragraph displays the correct account name
- Verifying the formatted address is displayed correctly
- Using
cryptoAmountWithSymbolOfto check the full amount with currency symbol- Adding validation that the fee is greater than zero
These checks ensure the user is presented with complete and accurate information during the transaction process.
packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.test.ts (2)
58-58: Good enhancement to the confirmTrade method.The method signature update to include the account name ('Solana #1') as the first parameter improves test clarity and strengthens verification. This change ensures that we're explicitly validating which account is being used for the transaction.
82-85: Great addition of navigation verification.Adding this test step ensures the user is properly returned to the account buy form after completing the transaction. This improves the end-to-end test coverage by verifying the full user journey.
packages/suite-desktop-core/e2e/tests/trading/sell-solana.test.ts (5)
11-11: Appropriate import addition.Good addition of the formatAddress import to standardize address formatting across test files.
25-25: Good reuse of formatAddress function.Creating a formatted address constant improves code readability and ensures consistent address formatting throughout the test.
90-95: Excellent enhancement to device prompt verification.These new assertions significantly improve test quality by verifying critical elements:
- Account name displayed in header
- Properly formatted address
- Accurate crypto amount with correct symbol
- Non-zero fee amount
This comprehensive verification helps catch potential UI and formatting issues early.
115-118: Good additional verification after status change.These assertions ensure the transaction details remain consistent after the trade status changes to success, providing better confidence in the UI state management.
120-123: Great addition of navigation verification.This test step verifies the complete user journey by ensuring they can return to the sell form after completing a transaction. The URL pattern check confirms navigation to the correct network and account.
packages/suite-desktop-core/e2e/tests/trading/buy-ethereum.test.ts (5)
61-66: Excellent verification of account selection options.Testing the visibility of both Ethereum accounts ensures the account selection dropdown is functioning correctly and displaying all available options to users.
70-70: Good verification of the device prompt header.Checking that the header correctly shows 'Ethereum #1' ensures the prompt is displaying the correct account information to users during confirmation.
84-84: Improved clarity with direct button interaction.Using a direct button click instead of a commented approach streamlines the test code and makes the flow more explicit.
87-94: Excellent addition of transaction detail verification.These assertions comprehensively validate the transaction status and details after confirmation, ensuring all values are displayed correctly to the user.
96-100: Great addition of navigation verification with useful comment.The test step ensures proper navigation after transaction completion. The comment on line 98 highlights a potential design consideration about which account's buy form should be displayed after completion.
This is a good question worth investigating. Should the user be returned to BTC (where the flow started) or ETH (which was bought)? Consider discussing this with the product team to confirm the intended behavior.
packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts (4)
59-59: Good enhancement to the confirmTrade method.Adding the account name parameter ('Ethereum #1') enhances verification specificity and ensures consistency with other trading tests.
82-87: Excellent enhancement to device prompt verification.These improved assertions provide more comprehensive validation:
- Account name in header
- Correctly formatted address
- Crypto amount with proper symbol formatting
- Non-zero fee verification
These changes align with the verification improvements in other trading test files.
95-103: Great addition of detailed confirmation verification.These assertions thoroughly verify all aspects of the swap transaction after approval, including:
- Send amount
- Receive amount
- Exchange type
- Provider information
This comprehensive verification ensures a complete and accurate user experience.
105-108: Good addition of navigation verification.Like the other trading tests, this step ensures the full user journey is verified by checking proper navigation back to the account swap form with an appropriate URL pattern.
packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts (6)
26-26: Good extraction of formatted address into a variable.The extracted variable improves code readability by making it clear what data is being passed to the
confirmTrademethod.
49-49: Method signature update looks good.The change to accept a parameter object with
symbolproperty improves the method's flexibility and makes the code more self-documenting.
68-68: Improved trade confirmation with explicit account name.The updated method call now explicitly passes the account name ('Bitcoin #1') along with the formatted address, which aligns with the updated method signature in the TradingPage class.
91-96: Enhanced verification with additional assertions.The changes now:
- Use
cryptoAmountWithSymbolOfto verify the amount with the symbol- Add a validation that the fee amount is greater than zero
These improvements provide more thorough validation of the trading process.
107-113: Excellent addition of partner support link verification.This new test step validates an important user flow - ensuring that the provider support link opens in a new tab with the correct URL pattern, which improves the test coverage.
126-135: Comprehensive transaction value verification added.The new test step thoroughly verifies all transaction values after the status changes, including:
- Crypto amounts for both sent and received currencies
- Exchange type
- Provider information
This adds significant value to the test coverage.
packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts (2)
16-17: Good separation of cryptocurrency amount getters.The rename of the original method to
cryptoAmountWithSymbolOfand addition of a newcryptoAmountOfmethod improves the API by clearly differentiating between getting:
- Amount with symbol (e.g., "0.01 BTC")
- Amount only (e.g., "0.01")
This allows for more precise testing.
Also applies to: 20-21
26-27: Useful header element accessors added.The new properties
headerandheaderParagraphwith their initializations provide access to important UI elements in the modal, enhancing the ability to validate header content during tests.Also applies to: 39-40
packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts (3)
46-46: Updated method call aligns with the new signature.The
confirmTrademethod call now includes the account name ('Bitcoin #1') as the first parameter, which correctly aligns with the updated method signature in the TradingPage class.
60-60: Consistent method call update.Similar to the previous comment, this method call has been updated to match the new signature, maintaining consistency throughout the test file.
91-94: Excellent addition of account redirect verification.This new test step verifies an important user flow - ensuring that after completing a transaction, users can return to their account page via the "Back to Account" button. This enhances the overall test coverage of the user experience.
packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts (2)
99-99: Good addition of new UI element accessors.The new properties and their initializations provide access to important UI elements:
backToAccountButton: Enables verification of navigation back to the account pageconfirmationExchangeType: Allows checking the type of exchange (e.g., fixed-rate)confirmationTransactionId: Provides access to the transaction identifiercopyTransactionIdButton: Enables testing of the copy functionalityThese additions enhance the test coverage for the trading pages.
Also applies to: 113-115, 174-174, 194-198
365-368: Improved trade confirmation with account name validation.The method signature update and the new assertion provide more thorough validation by:
- Requiring an explicit account name parameter (making it non-optional)
- Verifying that the header paragraph in the device prompt contains the specified account name
This ensures that trades are being confirmed for the correct account, which is an essential check.
packages/suite-desktop-core/e2e/fixtures/invity/swap/trade-solana-usdc.json
Show resolved
Hide resolved
c7a29cc to
c9dd532
Compare
Description
feat(e2e): Expands verification of our Trading tests based on feedback:
Related Issue