E2E: Check receive addr on emulator #17500
Conversation
WalkthroughThe changes span multiple files within the project’s end-to-end testing suite. In the core page objects, the ✨ Finishing Touches
🪧 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 (
|
43eca0c to
c9bf981
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts (3)
30-41: Well-structured display content comparison functionThis helper function encapsulates the common logic needed for all display-related matchers, avoiding code duplication. The use of lodash's
isEqualensures proper deep comparison of nested objects.Consider adding a more specific type for
expectedContentrather thananyto improve type safety.-const compareDisplayContent = async ( - devicePrompt: DevicePrompt, - expectedContent: any, - errorMessage: string, +const compareDisplayContent = async ( + devicePrompt: DevicePrompt, + expectedContent: { header: any; body: any[]; footer: string }, + errorMessage: string,
49-70: Comprehensive address transformation function with excellent documentationThe
transformAddressfunction is well-documented with clear comments explaining the different address formatting modes. The implementation handles both modes appropriately.However, the function doesn't handle invalid modes. While the type system should prevent this at compile time, it would be safer to add a fallback or error case.
const transformAddress = (address: string, mode: TransformMods = 'fourTetragrams') => { // ... existing code ... if (mode === 'fourTetragrams') { return addNewlinesToAddress(formatAddress(address), fourTetragramsOfAddress, ' \n'); } if (mode === 'fullLine') { return addNewlinesToAddress(address, fullLineOfAddress, ' \n '); } + + // Fallback to fourTetragrams if somehow an invalid mode is provided + return addNewlinesToAddress(formatAddress(address), fourTetragramsOfAddress, ' \n'); };
161-173: New matcher for validating transaction summary displayThis matcher handles the validation of transaction summaries, including total amount and fee information. The expected content structure accurately represents the device display format.
The structure of the body array is somewhat complex (array of arrays). Consider adding a comment explaining this structure to aid future maintenance.
async toDisplaySummary(devicePrompt: DevicePrompt, totalAmount: string, feeAmount: string) { + // Body is represented as an array of text segments, where each segment is an array + // This matches the display's internal representation of content const expectedContent = { header: { title: 'Summary' }, body: [['Total amount'], [totalAmount], [' '], ['incl. Transaction fee'], [feeAmount]], footer: 'Swipe up', };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts(1 hunks)packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts(2 hunks)packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts(5 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase-cardano.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase-reconnection.test.ts(3 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase.test.ts(5 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.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)packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
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/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.
⏰ 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: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (36)
packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts (1)
61-61: Simplified address handling improves test clarityThe change removes the
formatAddresscall, passing the rawreceiveAddressdirectly toconfirmTrade. This aligns with the PR objective of introducing a custom matcher for address validation, moving the formatting logic out of individual test cases.packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts (2)
60-60: Streamlined address handling consistent with PR goalsThe change removes the intermediate
formattedReceiveAddressvariable, passing the rawreceiveAddressdirectly toconfirmTrade. This is consistent with the changes in other trading tests, centralizing the address formatting logic.
74-74: Verification step is already using the raw addressGood to see the test was already verifying against the raw
receiveAddressrather than the formatted version, which makes this refactoring cleaner and more consistent.packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.test.ts (1)
57-57: Address handling refactored for consistencyThe change removes the
formatAddresscall, passing the rawreceiveAddressdirectly toconfirmTrade. This is consistent with the changes in other trading tests and supports the new address validation approach.packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts (2)
69-69: Simplified address handling improves test maintenanceThe change removes the
formatAddresscall, passing the rawreceiveAddressdirectly toconfirmTrade. This is consistent with the changes in other trading tests and centralizes address formatting logic.
74-74: Verification already uses raw address - good consistencyThe test already verifies against the raw
receiveAddress, making the refactoring of theconfirmTradecall more consistent with existing verification logic.packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts (2)
68-68: Address formatting simplifiedThe
formatAddressfunction call has been removed, and the rawreceiveAddressis now passed directly to theconfirmTrademethod. This simplifies the code and aligns with the PR's objective of improving address verification.
82-82: Address formatting simplifiedSimilar to the previous instance, the raw
receiveAddressis now passed directly to theconfirmTrademethod without formatting.packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts (1)
60-60: Address formatting simplifiedThe
formatAddressfunction call has been removed, and the rawreceiveAddressis now passed directly to theconfirmTrademethod, consistent with the changes in other trading test files.packages/suite-desktop-core/e2e/tests/passphrase/passphrase-reconnection.test.ts (3)
1-1: Added formatAddress importAdded import for the
formatAddressfunction which is now used for address formatting throughout the test.
41-43: Enhanced address verificationThe address verification has been improved with:
- Using
formatAddressto format the address consistently- Explicitly checking that the device prompt is shown
- Using the new custom matcher
toDisplayReceiveAddressto verify the address display on the emulatorThis aligns with the PR objective of enhancing address verification on the emulator.
70-72: Enhanced address verificationSimilar to the previous instance, the address verification has been improved with the same pattern of formatting, confirmation prompt checking, and using the new custom matcher.
packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts (3)
2-2: Added formatAddress importAdded import for the
formatAddressfunction used for address formatting in the test.
5-6: Simplified variable declarationReplaced
formattedReceiveAddresswithreceiveAddressusing the raw Cardano address, which is more consistent with the approach in other tests.
57-63: Enhanced Cardano address verificationThe verification process for Cardano addresses has been significantly improved:
- Using
devicePrompt.confirmOnDevicePromptIsShown()to verify the prompt is displayed- Using the new custom matcher
toDisplayReceiveAddresswith 'fullLine' transform mode for Cardano addresses- Adding explicit user confirmation with
trezorUserEnvLink.pressYes()- Using
formatAddressfor consistent address formattingThese changes enhance the test robustness by verifying both the UI display and the emulator display of the address.
packages/suite-desktop-core/e2e/tests/passphrase/passphrase-cardano.test.ts (4)
1-1: Good addition of formatAddress importImporting the
formatAddressutility function is a good replacement for the removedsplitStringEveryNCharactersimport, providing a more standardized way to format addresses for display and verification.
4-5: Updated address string looks goodThe updated Cardano address is longer and more realistic, which should improve test robustness for address display verification.
59-61: Improved address verification approachReplacing
toContainTextwithtoHaveTextand using the standardizedformatAddressfunction improves consistency and readability in address verification.
63-65: Great addition of emulator display verificationThis new assertion directly verifies that the address is correctly displayed on the device emulator, which aligns perfectly with the PR objective of enhancing e2e testing to check receive addresses on the emulator.
packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts (2)
90-96: Useful debugging method for display JSONThis helper method provides a convenient way to quickly access and debug the JSON content from the device display during test development, which will help troubleshoot issues with display assertions.
98-123: Well-structured display content extraction implementationThe implementation nicely handles:
- Retrieving the debug state from the device
- Parsing and validating the JSON structure
- Throwing descriptive errors for invalid content
- Extracting and organizing the relevant display sections
This method forms the foundation for the new display verification assertions and includes helpful comments about future extensibility.
packages/suite-desktop-core/e2e/tests/passphrase/passphrase.test.ts (5)
3-3: Good adoption of formatAddress utilityUsing the standardized
formatAddressutility instead ofsplitStringEveryNCharactersimproves consistency across tests.
26-26: Good addition of devicePrompt parameterAdding the
devicePromptparameter to the test context allows for verification of address display on the emulator.
46-48: Improved address verification with emulator display checkThe test now properly verifies that:
- The UI shows the correctly formatted address
- The device prompt is displayed
- The device displays the expected receive address
This multi-layered verification approach improves test reliability.
73-75: Consistent verification pattern for second addressApplying the same verification pattern to the second address check maintains consistency throughout the test.
93-95: Consistent verification pattern for third addressThe verification pattern is consistently applied to the third address check as well, demonstrating good test design.
packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts (3)
8-8: Good import of formatAddress utilityAdding
formatAddressto the imports ensures consistent address formatting across different components of the application.
369-369: Improved verification flow in confirmTrade methodAdding the device prompt confirmation check before other assertions ensures proper sequencing of verification steps.
371-374: Enhanced address verification in trading flowThe updated implementation:
- Verifies the UI shows the correctly formatted address using
formatAddress- Adds verification that the device properly displays the receive address
This brings trading flow address verification in line with the enhanced testing approach used in other tests.
packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts (7)
3-3: Import from lodash for deep comparisonThe import of
isEqualfrom lodash is appropriate for performing deep comparisons between complex objects in the new matcher functions.
5-6: Good addition of imports from internal modulesThese imports are needed for the new matchers and helper functions. The
formatAddressfunction is reused from common utilities, promoting code reuse.
8-8: Clear type definition for transformation modesThe
TransformModstype clearly defines the two supported address transformation modes that are used throughout the code.
43-48: Utility function for address formatting with newlinesThis helper function correctly handles the address formatting by adding newlines according to the provided regex pattern. The implementation is concise and focused.
127-144: New matcher for validating receive address displayThis matcher cleanly implements the functionality described in the PR objectives for verifying the display of receive addresses on the emulator. The function handles address transformation properly and creates the expected structure for comparison.
146-159: New matcher for validating recipient address displaySimilar to the receive address matcher but with appropriate adjustments for the recipient address display structure. The code reuses the existing transformation logic, maintaining consistency.
30-173: Consider adding tests for the new matcher functionsThe PR adds several new matchers and utility functions that are critical for validating device displays. While these are part of the testing framework themselves, consider adding unit tests specifically for these functions to ensure they transform addresses and compare content correctly in all edge cases.
For example, test cases could verify:
- Address transformation with different formats and lengths
- Content comparison with varying structures
- Error messages when comparisons fail
This would help ensure the reliability of these matchers when used in actual test scenarios.
| async toDisplayRecipientAddress(devicePrompt: DevicePrompt, expectedAddress: string) { | ||
| const transformedExpectedAddress = transformAddress(expectedAddress); | ||
| const expectedContent = { | ||
| header: { title: 'Address', subtitle: 'Recipient #1' }, | ||
| body: [transformedExpectedAddress], | ||
| footer: 'Swipe up', | ||
| }; | ||
|
|
||
| return await compareDisplayContent( | ||
| devicePrompt, | ||
| expectedContent, | ||
| 'expect Recipient address to match', | ||
| ); | ||
| }, | ||
|
|
||
| async toDisplaySummary(devicePrompt: DevicePrompt, totalAmount: string, feeAmount: string) { | ||
| const expectedContent = { | ||
| header: { title: 'Summary' }, | ||
| body: [['Total amount'], [totalAmount], [' '], ['incl. Transaction fee'], [feeAmount]], | ||
| footer: 'Swipe up', | ||
| }; | ||
|
|
||
| return await compareDisplayContent( | ||
| devicePrompt, | ||
| expectedContent, | ||
| 'expect Summary to match', | ||
| ); | ||
| }, |
There was a problem hiding this comment.
These two matchers are not used yet.
| await tradingPage.confirmTrade('Bitcoin #1', formatAddress(receiveAddress)); | ||
| await tradingPage.confirmTrade('Bitcoin #1', receiveAddress); |
There was a problem hiding this comment.
This is to unify in what form address is expected in toDisplayReceiveAddress parameter. It is just a address string without any formating.
| const formattedReceiveAddress = | ||
| 'addr _tes t1qp hsv6 vspp 4l3n vmqz w529 teq2 ha08 s0fg jvzg hzh6 28uc cfey 0wtr gp5r mxvl d7kh c745 x9mk 7gts 5ctu zerl f4ed rq5a t0x5'; | ||
| const receiveAddress = | ||
| 'addr_test1qphsv6vspp4l3nvmqzw529teq2ha08s0fgjvzghzh628uccfey0wtrgp5rmxvld7khc745x9mk7gts5ctuzerlf4edrq5at0x5'; |
There was a problem hiding this comment.
The toDisplayReceiveAddress expected address without any formating or newlines
| if (addressToCheck) { | ||
| await expect(this.devicePrompt.outputValueOf('address')).toHaveText(addressToCheck); | ||
| await expect(this.devicePrompt.outputValueOf('address')).toHaveText( | ||
| formatAddress(addressToCheck), | ||
| ); | ||
| await expect(this.devicePrompt).toDisplayReceiveAddress(addressToCheck); | ||
| } |
There was a problem hiding this comment.
await expect(this.devicePrompt).toDisplayReceiveAddress(addressToCheck); expects address parameter without any formating. So I have modified this method.
c9bf981 to
454750d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/suite-desktop-core/e2e/tests/passphrase/passphrase-reconnection.test.ts (2)
41-43: Use explicit transform mode for clarity
The default transform mode is'fourTetragrams'. If your address is expected to appear in a specific layout, consider specifying'fullLine'for consistency.
70-72: Extract repeated address checks
These lines duplicate the verification pattern used above. Consider creating a helper function for maintainability and to limit future drift between identical checks.packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts (1)
5-6: Encourage centralized test data
This explicit address constant is fine, but if it's reused elsewhere, consider moving it into a shared constants file to reduce duplication.packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts (1)
90-96: Avoid using errors for debugging
debugJSONFromDisplay()ends the test by throwing an error. If you want to inspect the output but keep the test running, consider console logging instead.packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts (2)
8-8: Potential extension
TransformModscurrently supports'fourTetragrams' | 'fullLine'. If more display modes arise, consider expanding this to avoid conditionals scattered throughout the code.
147-151: Expand transform options for recipient
toDisplayRecipientAddress()defaults to'fourTetragrams'. If you need'fullLine'or future formats, consider adding a transform parameter as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts(1 hunks)packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts(1 hunks)packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts(5 hunks)packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts(2 hunks)packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts(2 hunks)packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts(4 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase-cardano.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase-reconnection.test.ts(3 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase.test.ts(5 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.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)packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts
- packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts
- packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts
- packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts
- packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts
- packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts
- packages/suite-desktop-core/e2e/tests/passphrase/passphrase-cardano.test.ts
- packages/suite-desktop-core/e2e/tests/passphrase/passphrase.test.ts
- packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts
- packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts
- packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (9)
packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts (4)
24-24: No concerns
This line looks good as-is.
57-59: Informative transform mode
Specifying'fullLine'clarifies the expected device layout. This is a good example of explicit test behavior.
61-62: Check whitespace consistency
formatAddress(receiveAddress)might not align perfectly with'fullLine'in some edge cases. Verify that the displayed spacing remains correct.Would you like me to generate a script to compare both outputs and confirm no unintended whitespace differences?
2-2:✅ Verification successful
Validate cross-file usage
EnsureformatAddressyields an address string that's compatible with your device prompt checks. If you rely on'fullLine'mode in other tests, verify there's no mismatch.
🏁 Script executed:
#!/bin/bash # Search for occurrences of formatAddress usage rg -A 2 "formatAddress"Length of output: 13656
Confirmed Cross-File Consistency for
formatAddress
The usage offormatAddressinpackages/suite-desktop-core/e2e/tests/wallet/cardano.test.tsis consistent with its use in other test files. Its output, which enriches the address string by splitting into 4-character segments, aligns with the expected formatting for device prompt validations. There’s no indication of a mismatch with any "'fullLine' mode" requirements. Please continue to monitor related tests, ensuring that if any tests demand an uninterrupted address line, corresponding adjustments are made.packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts (1)
98-118: Validate single-paragraph assumption
getDisplayContent()throws if more than one paragraph is present. Ensure no future scenarios require multiple paragraphs.packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts (4)
5-5: Confirm usage of formatAddress
BecausetransformAddressmay already callformatAddress, double-check that the extra import here does not introduce duplicate or conflicting logic.
43-48: Regex-based edge cases
addNewlinesToAddressmight behave unexpectedly if the address includes unusual spacing or partial chunk sizes. Make sure these cases are tested.
49-70: Ensure consistent formatting
For'fourTetragrams', you callformatAddress. For'fullLine', you rely on the raw string. Verify that this discrepancy is intentional and doesn't introduce conflicting output.
127-131: Flexible transform mode
AcceptingtransformModehere provides useful versatility for verifying different display layouts.
now support different modes of transformation refactoring
minor refactoring
454750d to
441672f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/suite-desktop-core/e2e/tests/passphrase/passphrase-reconnection.test.ts (2)
41-43: Consider consolidating repeated test steps into a helper.
Lines 41-43 seem nearly identical to lines 70-72. A dedicated helper function (e.g.,verifyAddressPrompt(abcAddr)) could reduce duplication and improve maintainability.
70-72: Repeated logic for address validation.
This block repeats the same three-step sequence as lines 41-43. Refactoring into a shared utility or helper would simplify future updates and testing.packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts (2)
90-96: Handle potential parse errors in the debug method.
IfdebugState.tokensare not valid JSON, this will crash abruptly. Consider wrappingJSON.parsein a try/catch or validating the input to avoid uncaught parse errors.
98-123: Use typed structures or robust checks for JSON structure.
Accessingjson.content.content.paragraphscan break if the structure changes. Defining a clear interface or adding fallback checks would improve maintainability.packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts (1)
24-24: Remove or clarify commented-out lines if no longer needed.
Leaving stale comments can cause confusion. Consider deleting or explaining the reason for keeping them commented out.Also applies to: 27-27, 31-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/suite-desktop-core/e2e/support/pageObjects/devicePrompt.ts(1 hunks)packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts(2 hunks)packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts(5 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase-cardano.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase-reconnection.test.ts(3 hunks)packages/suite-desktop-core/e2e/tests/passphrase/passphrase.test.ts(5 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts(2 hunks)packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.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)packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/suite-desktop-core/e2e/tests/trading/swap-tokens.test.ts
- packages/suite-desktop-core/e2e/tests/trading/swap-token-to-coin.test.ts
- packages/suite-desktop-core/e2e/tests/trading/buy-bitcoin.test.ts
- packages/suite-desktop-core/e2e/tests/trading/buy-solana-token.test.ts
- packages/suite-desktop-core/e2e/tests/trading/swap-coin-to-token.test.ts
- packages/suite-desktop-core/e2e/tests/passphrase/passphrase-cardano.test.ts
- packages/suite-desktop-core/e2e/tests/trading/swap-coins.test.ts
- packages/suite-desktop-core/e2e/tests/passphrase/passphrase.test.ts
- packages/suite-desktop-core/e2e/support/pageObjects/tradingPage.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (12)
packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts (4)
2-2: No issues with the new import.
The import offormatAddressaligns with the test’s formatting requirements.
5-6: Initialization of the receive address looks good.
Defining a straightforward constant for the test ensures clarity for Cardano address verification.
57-62: Clear step sequence for verifying 'Legacy Testnet' address.
Displaying the prompt, checking withtoDisplayReceiveAddress, and pressing 'Yes' is logically consistent. No issues.
64-65: Follow-up address confirmation appears consistent.
Verifying the output value and re-checking the prompt is a good approach to ensure the correct address is still displayed.packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts (8)
3-3: “lodash/isEqual” import is appropriate.
Deep equality checks are beneficial for comparing structured test objects.
5-6: Import of additional utilities from common and DevicePrompt recognized.
These imports support new matchers and align with the broader test framework updates.
8-8: Definition of “LineFormats” is clear.
Using a union type for address formatting modes is straightforward and expressive.
30-42: compareDisplayContent function logically compares expected vs. actual.
UsesdevicePrompt.getDisplayContentandisEqual; the diff in error messages is helpful for debugging.
43-48: addNewlinesToAddress function is concise.
Trimming and splitting by regex is an efficient approach for device-side text formatting checks.
127-156: toDisplayReceiveAddress matches the new device JSON structure.
Headers, body, and footers are well-handled with fallback to 'Swipe up' or 'Tap to continue' for different contexts.
158-171: toDisplayRecipientAddress ensures clarity for multi-recipient flows.
Subtitle “Recipient #1” is explicit. The approach is coherent with the reusability of address formatting logic.
173-185: toDisplaySummary thoroughly checks total amount and fee.
This matcher enforces a consistent summary layout, improving test reliability.
| const transformAddress = (address: string, lineFormat: LineFormats = 'fourTetragrams') => { | ||
| // Address is split to lines on Display so it can fit. There are different formats: | ||
| // 1. Four tetragrams of address: | ||
| // bc1q pyfv fvm5 2zx7 | ||
| // gek8 6ajj 5pkk ne3h | ||
| // 385a da8r 2y | ||
| // 1. Full lines (18 chars) of address: | ||
| // bc1qpyfvfvm52zx7ge | ||
| // k86ajj5pkkne3h385a | ||
| // da8r2y | ||
| // We want to evaluate format and existence of newlines in the address. | ||
| const fourTetragramsOfAddress = /(\S+\s\S+\s\S+\s\S+)/g; //4 x 4 characters | ||
| const fullLineOfAddress = /.{18}/g; //18 characters | ||
|
|
||
| if (lineFormat === 'fourTetragrams') { | ||
| return addNewlinesToAddress(formatAddress(address), fourTetragramsOfAddress, ' \n'); | ||
| } | ||
|
|
||
| if (lineFormat === 'fullLine') { | ||
| return addNewlinesToAddress(address, fullLineOfAddress, ' \n '); | ||
| } | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify omission of formatAddress in 'fullLine'.
“fourTetragrams” utilizes formatAddress, but “fullLine” does not. If that is intentional, all good. Otherwise, confirm consistency or consider unifying.
🏁 Script executed:
#!/bin/bash
# Verify usage of “fullLine” vs. “fourTetragrams” across the codebase to ensure consistency and no overlooked calls
rg 'lineFormat: .fullLine' -A 5
rg 'lineFormat: .fourTetragrams' -A 5Length of output: 2099
Inconsistency in Address Formatting for "fullLine" vs. "fourTetragrams"
It appears that in the transformAddress function (file: packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts), the "fourTetragrams" branch applies formatAddress(address) before adding newlines, whereas the "fullLine" branch does not call formatAddress. However, tests (e.g., in packages/suite-desktop-core/e2e/tests/wallet/cardano.test.ts and packages/suite-desktop-core/e2e/tests/passphrase/passphrase-cardano.test.ts) expect the output to match formatAddress(receiveAddress) when lineFormat is set to "fullLine".
- Issue:
"fullLine"currently omitsformatAddress, leading to an inconsistency with the"fourTetragrams"branch. - Action Required: Confirm whether this omission is intentional. If not, consider applying
formatAddressin the"fullLine"branch (or update the tests as needed) to ensure that both formatting modes behave consistently.
There was a problem hiding this comment.
Yes it is intentional. formatAddress() transform address to tetragrams
There was a problem hiding this comment.
@Vere-Grey Thanks for the clarification! I understand now - formatAddress() is specifically for transforming addresses into tetragrams, which is needed only for the 'fourTetragrams' format. For the 'fullLine' format, you're deliberately using the raw address since it's meant to display full lines without tetragram formatting.
This design approach makes perfect sense for handling the different display modes on the emulator.
✏️ Learnings added
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17500
File: packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts:49-70
Timestamp: 2025-03-12T10:05:27.207Z
Learning: In the `transformAddress` function used for e2e testing in Trezor Suite, `formatAddress()` is intentionally applied only to the 'fourTetragrams' format (which displays addresses in groups of 4 characters), and not to the 'fullLine' format (which displays addresses in lines of up to 18 characters without tetragram grouping).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| // await settingsPage.coins.openNetworkAdvanceSettings('tada'); | ||
| // await expect(settingsPage.modal).toHaveScreenshot('cardano-advanced-settings.png', { | ||
| // mask: [settingsPage.coins.coinAddressInput], | ||
| // }); | ||
| await settingsPage.modalCloseButton.click(); | ||
| // await settingsPage.modalCloseButton.click(); |
There was a problem hiding this comment.
These steps were here just for the toHaveScreenshot
| if (options.specialAccountType) { | ||
| expectedContent = { | ||
| header: { title: 'Receive address' }, | ||
| body: [[options.specialAccountType], transformedExpectedAddress], | ||
| footer: 'Tap to continue', | ||
| }; | ||
| } else { | ||
| expectedContent = { | ||
| header: { title: 'Receive address' }, | ||
| body: [transformedExpectedAddress], | ||
| footer: 'Swipe up', | ||
| }; |
There was a problem hiding this comment.
I don't know if there will be more special ways of displaying the information.
Maybe it is now worth to validate the address on test net and complicate this method?
We have other cardano test on normal net with this validation 🤔
|
Seeing a bunch of issues like this |
This PR introduces first asserts of what is actually displayed on emulator to our e2e tests. So far we checked values only on prompt in the suite. Now we have custom Matcher
toDisplayReceiveAddresswhich is to be called on expect(devicePrompt object) and takes parameter of address and optional parameter of tranformMode. The optional parameter is there because I have encountered two ways how address is displayed on the emulator.The assert is used in every test operating with receive address.
I have added also two other asserts but their use would be complicated now. Because of how we resuse some methods in Trading tests. I need rethink that and choose correct approach. Also I dont want this PR to be too big.