feat(e2e): Migrated suite tests #16934
Conversation
WalkthroughThe pull request introduces several updates across desktop and web packages. In the desktop core, new public properties and methods have been added to various classes to support interactions with additional UI elements, such as toggle switches, log buttons, guide-related elements, and safety check controls. Additionally, multiple end-to-end test suites have been created to validate functionality around the Bridge page, bug report forms, guide interactions, application initial run behavior, safety checks warnings, and version page verification. Conversely, corresponding test files in the web package have been removed, eliminating tests for similar functionalities in that environment. The changes include new test setup hooks to manage environment conditions—such as stopping emulators and resetting settings—and the introduction of assertions to ensure expected UI behavior and state management across the application. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (15)
💤 Files with no reviewable changes (6)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (2)
✨ 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 (
|
![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
🧹 Nitpick comments (13)
packages/suite-desktop-core/e2e/tests/suite/bridge.test.ts (1)
10-18: Consider adding negative test cases.While the happy path is covered, consider adding test cases for:
- WebUSB not supported
- Bridge installation required
packages/suite-desktop-core/e2e/tests/suite/version-page.test.ts (1)
12-17: Consider enhancing version verification.The test verifies the version number display but could be enhanced:
- Verify other version page elements
- Add error cases (e.g., version mismatch)
- Consider removing or documenting the purpose of the screenshot
packages/suite-desktop-core/e2e/tests/suite/bug-report-form.test.ts (2)
8-11: Consider parameterizing test data.The test data could be moved to a separate fixture or data provider to:
- Test multiple feedback categories
- Test various report content lengths
13-18: Add error handling test cases.While the happy path is covered, consider adding test cases for:
- Empty report submission
- Network failures
- Rate limiting
packages/suite-desktop-core/e2e/tests/suite/initial-run.test.ts (2)
8-21: Improve test name clarity and coverage.The test name "Until user passed through initial run, it will be there after reload" could be more descriptive. Consider renaming to better reflect the test's purpose of verifying analytics toggle visibility and persistence.
- test('Until user passed through initial run, it will be there after reload', async ({ + test('Analytics toggle remains visible after reload until user confirms choice', async ({Additionally, consider adding a test to verify that the analytics toggle state (enabled/disabled) persists after reload.
23-32: Fix typo in test name.The test name contains a typo: "trough" should be "through".
- test('Once user passed trough, skips initial run and shows connect-device modal', async ({ + test('Once user passed through, skips initial run and shows connect-device modal', async ({packages/suite-desktop-core/e2e/tests/suite/safety-checks-warning.test.ts (2)
4-8: Consider improving test isolation.The
beforeEachhook modifies global state by completing onboarding and changing safety check levels. Consider resetting the state after each test to ensure test isolation.Add an
afterEachhook to reset safety checks to default:test.afterEach(async ({ settingsPage }) => { await settingsPage.changeSafetyChecksLevel('strict'); // Reset to default });
10-13: Add assertion for warning message content.The test verifies the visibility of warning elements but doesn't check the actual warning message content.
Add an assertion to verify the warning message:
await expect(page.getByTestId('@banner/safety-checks/message')).toHaveText(/Safety checks are set to prompt/);packages/suite-desktop-core/e2e/tests/suite/guide.test.ts (2)
24-27: Add error handling test for feedback form.The feedback form is only tested for the success case. Consider adding tests for error scenarios.
Add test cases for:
- Empty feedback submission
- Network failure during submission
- Rate limiting
37-42: Extract search test data to constants.The search terms "trezor" and "meow-wuf-nonsense" are hardcoded. Consider extracting these to test constants for better maintainability.
const TEST_DATA = { VALID_SEARCH: 'trezor', INVALID_SEARCH: 'meow-wuf-nonsense' } as const;packages/suite-desktop-core/e2e/support/pageActions/suiteGuideActions.ts (2)
30-32: Add JSDoc comments for new properties.The new properties lack documentation explaining their purpose and usage.
+ /** Locator for guide navigation nodes */ readonly guideNodes: Locator; + /** Locator for guide section label */ readonly guideLabel: Locator; + /** Locator for empty search results message */ readonly searchNoResults: Locator;
71-78: Improve error handling in sendBugReport method.The method should handle potential failures during bug report submission.
@step() async sendBugReport(args: { location: FeedbackCategory; report: string }) { + try { await this.bugFormButton.click(); await this.selectBugLocation(args.location); // stability necessity await this.page.waitForTimeout(250); await this.bugInputTextField.fill(args.report); await this.submitButton.click(); + await expect(this.feedbackSuccessToast).toBeVisible({ timeout: 10000 }); + } catch (error) { + throw new Error(`Failed to send bug report: ${error.message}`); + } }packages/suite-desktop-core/e2e/support/pageActions/settings/settingsActions.ts (1)
206-214: Add error handling and success verification.The method implementation is good but could be enhanced with:
- Error handling for device confirmation
- Success notification verification (similar to other methods)
- JSDoc documentation for better maintainability
+/** + * Changes the safety checks level on the device + * @param level - The safety checks level to set ('strict' or 'prompt') + * @throws {Error} If device confirmation fails + */ @step() async changeSafetyChecksLevel(level: 'strict' | 'prompt') { await this.navigateTo('device'); await this.page.getByTestId('@settings/device/safety-checks-button').click(); await this.page.getByTestId(`@radio-button-${level}`).click(); await this.page.getByTestId('@safety-checks-apply').click(); await expect(this.confirmOnDevicePrompt).toBeVisible(); await TrezorUserEnvLinkProxy.pressYes(); + await this.confirmOnDevicePrompt.waitFor({ state: 'detached' }); + await expect(this.notificationSuccessToast).toBeVisible(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/suite-desktop-core/e2e/support/pageActions/analyticsActions.ts(1 hunks)packages/suite-desktop-core/e2e/support/pageActions/settings/settingsActions.ts(3 hunks)packages/suite-desktop-core/e2e/support/pageActions/suiteGuideActions.ts(2 hunks)packages/suite-desktop-core/e2e/tests/suite/bridge.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/suite/bug-report-form.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/suite/guide.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/suite/initial-run.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/suite/safety-checks-warning.test.ts(1 hunks)packages/suite-desktop-core/e2e/tests/suite/version-page.test.ts(1 hunks)packages/suite-web/e2e/tests/suite/bridge.test.ts(0 hunks)packages/suite-web/e2e/tests/suite/bug-report-form.test.ts(0 hunks)packages/suite-web/e2e/tests/suite/guide.test.ts(0 hunks)packages/suite-web/e2e/tests/suite/initial-run.test.ts(0 hunks)packages/suite-web/e2e/tests/suite/safety-checks-warning.test.ts(0 hunks)packages/suite-web/e2e/tests/suite/version-page.test.ts(0 hunks)
💤 Files with no reviewable changes (6)
- packages/suite-web/e2e/tests/suite/bug-report-form.test.ts
- packages/suite-web/e2e/tests/suite/safety-checks-warning.test.ts
- packages/suite-web/e2e/tests/suite/version-page.test.ts
- packages/suite-web/e2e/tests/suite/guide.test.ts
- packages/suite-web/e2e/tests/suite/bridge.test.ts
- packages/suite-web/e2e/tests/suite/initial-run.test.ts
⏰ 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=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: Analyze with CodeQL (javascript)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: build-web
🔇 Additional comments (3)
packages/suite-desktop-core/e2e/support/pageActions/analyticsActions.ts (1)
8-8: LGTM! Well-structured addition of the toggle switch locator.The new
toggleSwitchproperty follows the established patterns for test IDs and locator initialization.Also applies to: 13-13
packages/suite-desktop-core/e2e/tests/suite/bridge.test.ts (1)
4-9: LGTM! Proper test setup with clean environment.The
beforeEachhook ensures a clean test environment by stopping both emulator and bridge.packages/suite-desktop-core/e2e/support/pageActions/settings/settingsActions.ts (1)
69-69: LGTM!The
showLogButtonproperty follows the established patterns for locator properties in the class, with consistent naming and test ID conventions.Also applies to: 104-104
HajekOndrej
force-pushed
the
migrate-suite
branch
from
f7333cf to
684d8ae
Compare
Description
Migrated the tests in suite subfolder, that were not related to passphrase.
Related Issue
Resolve
Screenshots: