refactor(e2e): Refactor fixtures #17287
Conversation
Split it to suiteBase and pageObjects + mocks fixure file slimmed down metadataProviderMock.stop() moved to fixture teardown untangle page fixture
There was a problem hiding this comment.
Lot of fixtures moved to suiteBaseFixtures.ts:
all the config/boolean switches, url, trezorUserEnvLink, page, exceptionLogger
The electronWindow stopped existing.
Remaining fixtures here are page objects and mocks.
Fixtures extands here are based of the suiteBaseFixtures.ts so that they use the correct page object
| onboardingPage: async ( | ||
| { page, analyticsPage, devicePrompt, emulatorStartConf }, | ||
| use, | ||
| testInfo, | ||
| ) => { | ||
| const onboardingPage = new OnboardingActions( | ||
| page, | ||
| analyticsPage, | ||
| devicePrompt, | ||
| emulatorStartConf.model, | ||
| testInfo, | ||
| ); | ||
| await use(onboardingPage); | ||
| await use(new WalletActions(page)); | ||
| }, | ||
| onboardingPage: async ({ page, emulatorStartConf }, use, testInfo) => { | ||
| await use(new OnboardingActions(page, emulatorStartConf.model, testInfo)); | ||
| }, |
There was a problem hiding this comment.
the page object initilizes everything it needs and can to reduce passing of unnecessary params
There was a problem hiding this comment.
I am not entirely sure I like this. Some of the fixtures are not stateless and one could assume, that all of them are singletons. I know we have used initialization within the fixtures in the past as well, but I am not 100% sure that it is desired.
We can definitely use this approach, because I don't know what is right/wrong, I am just pointing it out as something to consider.
| }, | ||
| metadataProviderMock: async ({ page }, use) => { | ||
| const metadataProviderMock = new MetadataProviderMock(page); | ||
| await use(metadataProviderMock); | ||
| await metadataProviderMock.stop(); |
There was a problem hiding this comment.
global teardown to DRY tests
There was a problem hiding this comment.
Untangle the page object and all the setup.
also adjust multiple-session tests
There was a problem hiding this comment.
I had to adjust the test to change of how we start the web page now.
WalkthroughThe changes update the testing infrastructure and fixture definitions throughout the suite. A new type, ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/suite-desktop-core/e2e/support/common.ts (1)
109-114: Add error handling when launching the app.
IfelectronAppfails to launch orfirstWindow()is null, the function should handle it gracefully (e.g., throwing an informative error or retrying).packages/suite-desktop-core/e2e/support/pageActions/onboarding/backupActions.ts (1)
18-19: Consider using dependency injection for easier test mocking.
InstantiatingdevicePromptdirectly within the constructor reduces flexibility for mocking or substituting test doubles in complex scenarios.packages/suite-desktop-core/e2e/support/testExtends/suiteBaseFixture.ts (3)
28-39: Consider adding error handling for Electron launch failures.
For instance, throw a descriptive error if Electron fails to start, helping diagnose setup issues quickly.
57-68: Verify that the web application is ready before proceeding.
Optionally wait for a specific DOM element or route to ensure the page initialization is complete.
70-90: Consider verifying emulator readiness.
After starting or setting up the emulator, ensure it’s in a ready state (e.g., using a check call) to avoid race conditions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/suite-desktop-core/e2e/support/common.ts(3 hunks)packages/suite-desktop-core/e2e/support/fixtures.ts(3 hunks)packages/suite-desktop-core/e2e/support/pageActions/marketActions.ts(1 hunks)packages/suite-desktop-core/e2e/support/pageActions/onboarding/backupActions.ts(2 hunks)packages/suite-desktop-core/e2e/support/pageActions/onboarding/onboardingActions.ts(2 hunks)packages/suite-desktop-core/e2e/support/pageActions/settings/coinActions.ts(1 hunks)packages/suite-desktop-core/e2e/support/pageActions/settings/settingsActions.ts(1 hunks)packages/suite-desktop-core/e2e/support/pageActions/trezorInputActions.ts(1 hunks)packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts(1 hunks)packages/suite-desktop-core/e2e/support/testExtends/suiteBaseFixture.ts(1 hunks)packages/suite-desktop-core/e2e/tests/bridge-tor/spawn-bridge.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/account-metadata.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/address-metadata.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/dropbox-api-errors.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/google-api-errors.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/interval-fetching.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/output-labeling.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/remembered-device.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/metadata/wallet-metadata.test.ts(0 hunks)packages/suite-desktop-core/e2e/tests/suite/multiple-sessions.test.ts(2 hunks)
💤 Files with no reviewable changes (9)
- packages/suite-desktop-core/e2e/tests/metadata/address-metadata.test.ts
- packages/suite-desktop-core/e2e/tests/metadata/google-api-errors.test.ts
- packages/suite-desktop-core/e2e/tests/metadata/remembered-device.test.ts
- packages/suite-desktop-core/e2e/tests/metadata/output-labeling.test.ts
- packages/suite-desktop-core/e2e/tests/metadata/interval-fetching.test.ts
- packages/suite-desktop-core/e2e/tests/metadata/wallet-metadata.test.ts
- packages/suite-desktop-core/e2e/tests/metadata/account-metadata.test.ts
- packages/suite-desktop-core/e2e/tests/metadata/dropbox-api-errors.test.ts
- packages/suite-desktop-core/e2e/tests/bridge-tor/spawn-bridge.test.ts
✅ Files skipped from review due to trivial changes (5)
- packages/suite-desktop-core/e2e/support/testExtends/customMatchers.ts
- packages/suite-desktop-core/e2e/support/pageActions/trezorInputActions.ts
- packages/suite-desktop-core/e2e/support/pageActions/settings/coinActions.ts
- packages/suite-desktop-core/e2e/support/pageActions/marketActions.ts
- packages/suite-desktop-core/e2e/support/pageActions/settings/settingsActions.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/suite-desktop-core/e2e/support/testExtends/suiteBaseFixture.ts
[error] 25-25: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 101-101: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 105-105: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (31)
packages/suite-desktop-core/e2e/support/common.ts (2)
3-3: No issues found in this import statement.
28-32: Looks good.packages/suite-desktop-core/e2e/support/pageActions/onboarding/backupActions.ts (1)
7-8: No concerns with storing devicePrompt as a private member.packages/suite-desktop-core/e2e/tests/suite/multiple-sessions.test.ts (2)
6-6: Import statement verified.
88-109: Implementation looks solid.
The approach to open a second tab, set up onboarding, and test session overtaking is valid. No issues detected.packages/suite-desktop-core/e2e/support/testExtends/suiteBaseFixture.ts (4)
1-2: No further comments on ESLint override.
4-13: No issues with these imports.
41-55: No issues with tracing and teardown logic.
166-167: Export statement verified.packages/suite-desktop-core/e2e/support/pageActions/onboarding/onboardingActions.ts (3)
20-21: Good addition of private properties for clarity.
These private fields help encapsulate actions cleanly.
47-47: Use ofreadonly modelis consistent.
Passing themodelas areadonlyparameter is appropriate and ensures clarity within the class constructor.
50-52: Direct instantiation of dependencies is acceptable.
Creating instances foranalyticsPage,devicePrompt, andbackuphere is clear and keeps the dependency management straightforward for e2e tests.packages/suite-desktop-core/e2e/support/fixtures.ts (19)
19-19: Switching tosuiteBaseTestimproves test consistency.
This centralized approach simplifies fixture management.
41-41: Extension ofsuiteBaseTestwith typed fixtures is well-structured.
This pattern keeps the code strongly typed and easier to maintain.
43-43: PassingdevicePrompttoDashboardActionsis straightforward.
Ensures the dashboard actions can leverage device prompt interactions directly.
46-46:SettingsActionsusage looks clean.
No issues spotted with passingurl.
49-49:SuiteGuideinstantiation is simple and consistent.
Keeps the page object model approach uniform.
52-53:WalletActionsfixture extension appears correct.
No concerns with its direct usage.
54-55:onboardingPagereferencesmodelconsistently.
InjectingmodelandtestInfoaligns well with theOnboardingActionsconstructor.
58-58:AnalyticsActionsfixture usage is straightforward.
No concerns identified.
61-61:DevicePromptActionsfixture looks good.
Retaining consistent constructor parameters.
64-64:RecoveryActionsextension is aligned with the new fixture pattern.
67-67:MarketActionsusage is consistent.
No issues found.
70-70:AssetsActionsinstantiation is fine.
Allows direct user flows without extra parameters.
73-73:MetadataActionsnow receivesdevicePromptvia fixture.
Properly aligns with the rest of the refactor.
76-76:TrezorInputActionsfixture is straightforward.
No issues noted.
79-79:AnalyticsFixtureintegration is correct.
Continues the consistent pattern of direct page usage.
82-82:IndexedDbFixtureusage is properly extended.
No concerns.
87-87: Explicitly stoppingmetadataProviderMockis a good teardown practice.
Ensures no lingering resources remain.
96-97:TradingMockfixture is in line with the new structure.
Keeps everything consistent within the e2e approach.
101-101: Exportingexpectfrom custom matchers is consistent with the new approach.
Maintains a uniform testing API.
| type StartEmuModelRequired = StartEmu & { model: Model }; | ||
|
|
||
| type suiteBaseFixture = { | ||
| startEmulator: boolean; | ||
| setupEmulator: boolean; | ||
| emulatorStartConf: StartEmuModelRequired; | ||
| emulatorSetupConf: SetupEmu; | ||
| url: string; | ||
| trezorUserEnvLink: TrezorUserEnvLinkClass; | ||
| page: Page; | ||
| exceptionLogger: void; | ||
| }; |
There was a problem hiding this comment.
Replace the confusing “void” type with “undefined” or remove the property if unused.
Using “void” in this context can cause confusion and is flagged by linters.
- exceptionLogger: void;
+ exceptionLogger?: undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type StartEmuModelRequired = StartEmu & { model: Model }; | |
| type suiteBaseFixture = { | |
| startEmulator: boolean; | |
| setupEmulator: boolean; | |
| emulatorStartConf: StartEmuModelRequired; | |
| emulatorSetupConf: SetupEmu; | |
| url: string; | |
| trezorUserEnvLink: TrezorUserEnvLinkClass; | |
| page: Page; | |
| exceptionLogger: void; | |
| }; | |
| type StartEmuModelRequired = StartEmu & { model: Model }; | |
| type suiteBaseFixture = { | |
| startEmulator: boolean; | |
| setupEmulator: boolean; | |
| emulatorStartConf: StartEmuModelRequired; | |
| emulatorSetupConf: SetupEmu; | |
| url: string; | |
| trezorUserEnvLink: TrezorUserEnvLinkClass; | |
| page: Page; | |
| exceptionLogger?: undefined; | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
There was a problem hiding this comment.
Our type check does not agree with you.
There was a problem hiding this comment.
@Vere-Grey I understand - thanks for letting me know. Different projects often have specific type checking requirements that may differ from general linting rules. I'll respect your team's type system preferences for this codebase.
✏️ Learnings added
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17287
File: packages/suite-desktop-core/e2e/support/testExtends/suiteBaseFixture.ts:15-26
Timestamp: 2025-02-27T17:06:12.326Z
Learning: In the trezor-suite project, using `void` as a property type (as in `exceptionLogger: void;`) is acceptable according to the project's type checking requirements, even though some linters might flag it.
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.
| // This is the base Suite text fixture containing all the necessary setup and core page object | ||
| // Depending on the project type (desktop or web) it will launch the appropriate environment | ||
| // and provide the necessary page object which is either electron window or web page | ||
| const suiteBaseTest = base.extend<suiteBaseFixture>({ | ||
| startEmulator: true, | ||
| setupEmulator: true, | ||
| emulatorStartConf: { model: 'T3T1', wipe: true }, | ||
| emulatorSetupConf: {}, | ||
| /* eslint-disable-next-line no-empty-pattern */ | ||
| url: async ({}, use, testInfo) => { | ||
| await use(getUrl(testInfo)); | ||
| }, | ||
| /* eslint-disable-next-line no-empty-pattern */ | ||
| trezorUserEnvLink: async ({}, use) => { | ||
| await use(TrezorUserEnvLinkProxy); | ||
| }, | ||
| page: async ( | ||
| { | ||
| locale, | ||
| colorScheme, | ||
| browser, | ||
| startEmulator, | ||
| setupEmulator, | ||
| emulatorStartConf, | ||
| emulatorSetupConf, | ||
| }, | ||
| use, | ||
| testInfo, | ||
| ) => { | ||
| // This Trezor env setup needs to happen before electron or web page are launched | ||
| await trezorEnvSetup( | ||
| testInfo, | ||
| startEmulator, | ||
| setupEmulator, | ||
| emulatorStartConf, | ||
| emulatorSetupConf, | ||
| ); | ||
|
|
||
| if (isDesktopProject(testInfo)) { | ||
| const suite = await electronSetup(testInfo, locale, colorScheme); | ||
| await use(suite.window); | ||
| await electronTeardown(suite, testInfo); | ||
| } else { | ||
| const browserContext = await browser.newContext(); | ||
| const page = await webSetup(browserContext); | ||
| await use(page); | ||
| await page.close(); | ||
| await browserContext.close(); | ||
| } | ||
|
|
||
| await TrezorUserEnvLinkProxy.logTestDetails( | ||
| ` - - - FINISHING TEST ${testInfo.titlePath.join(' - ')}`, | ||
| ); | ||
| }, | ||
| exceptionLogger: [ | ||
| async ({ page }, use) => { | ||
| const errors: Error[] = []; | ||
| page.on('pageerror', error => { | ||
| errors.push(error); | ||
| }); | ||
|
|
||
| await use(); | ||
|
|
||
| if (errors.length > 0) { | ||
| throw new Error( | ||
| `There was a JS exception during test run. | ||
| \n${errors.map(error => `${error.message}\n${error.stack}`).join('\n-----\n')}`, | ||
| ); | ||
| } | ||
| }, | ||
| { auto: true }, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix lint warnings for empty object patterns on lines 101 and 105.
Avoid using empty destructuring if no parameters are needed:
- url: async ({}, use, testInfo) => {...}
+ url: async (_, use, testInfo) => {...}
- trezorUserEnvLink: async ({}, use) => {...}
+ trezorUserEnvLink: async (_, use) => {...}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // This is the base Suite text fixture containing all the necessary setup and core page object | |
| // Depending on the project type (desktop or web) it will launch the appropriate environment | |
| // and provide the necessary page object which is either electron window or web page | |
| const suiteBaseTest = base.extend<suiteBaseFixture>({ | |
| startEmulator: true, | |
| setupEmulator: true, | |
| emulatorStartConf: { model: 'T3T1', wipe: true }, | |
| emulatorSetupConf: {}, | |
| /* eslint-disable-next-line no-empty-pattern */ | |
| url: async ({}, use, testInfo) => { | |
| await use(getUrl(testInfo)); | |
| }, | |
| /* eslint-disable-next-line no-empty-pattern */ | |
| trezorUserEnvLink: async ({}, use) => { | |
| await use(TrezorUserEnvLinkProxy); | |
| }, | |
| page: async ( | |
| { | |
| locale, | |
| colorScheme, | |
| browser, | |
| startEmulator, | |
| setupEmulator, | |
| emulatorStartConf, | |
| emulatorSetupConf, | |
| }, | |
| use, | |
| testInfo, | |
| ) => { | |
| // This Trezor env setup needs to happen before electron or web page are launched | |
| await trezorEnvSetup( | |
| testInfo, | |
| startEmulator, | |
| setupEmulator, | |
| emulatorStartConf, | |
| emulatorSetupConf, | |
| ); | |
| if (isDesktopProject(testInfo)) { | |
| const suite = await electronSetup(testInfo, locale, colorScheme); | |
| await use(suite.window); | |
| await electronTeardown(suite, testInfo); | |
| } else { | |
| const browserContext = await browser.newContext(); | |
| const page = await webSetup(browserContext); | |
| await use(page); | |
| await page.close(); | |
| await browserContext.close(); | |
| } | |
| await TrezorUserEnvLinkProxy.logTestDetails( | |
| ` - - - FINISHING TEST ${testInfo.titlePath.join(' - ')}`, | |
| ); | |
| }, | |
| exceptionLogger: [ | |
| async ({ page }, use) => { | |
| const errors: Error[] = []; | |
| page.on('pageerror', error => { | |
| errors.push(error); | |
| }); | |
| await use(); | |
| if (errors.length > 0) { | |
| throw new Error( | |
| `There was a JS exception during test run. | |
| \n${errors.map(error => `${error.message}\n${error.stack}`).join('\n-----\n')}`, | |
| ); | |
| } | |
| }, | |
| { auto: true }, | |
| ], | |
| }); | |
| // This is the base Suite text fixture containing all the necessary setup and core page object | |
| // Depending on the project type (desktop or web) it will launch the appropriate environment | |
| // and provide the necessary page object which is either electron window or web page | |
| const suiteBaseTest = base.extend<suiteBaseFixture>({ | |
| startEmulator: true, | |
| setupEmulator: true, | |
| emulatorStartConf: { model: 'T3T1', wipe: true }, | |
| emulatorSetupConf: {}, | |
| /* eslint-disable-next-line no-empty-pattern */ | |
| url: async (_, use, testInfo) => { | |
| await use(getUrl(testInfo)); | |
| }, | |
| /* eslint-disable-next-line no-empty-pattern */ | |
| trezorUserEnvLink: async (_, use) => { | |
| await use(TrezorUserEnvLinkProxy); | |
| }, | |
| page: async ( | |
| { | |
| locale, | |
| colorScheme, | |
| browser, | |
| startEmulator, | |
| setupEmulator, | |
| emulatorStartConf, | |
| emulatorSetupConf, | |
| }, | |
| use, | |
| testInfo, | |
| ) => { | |
| // This Trezor env setup needs to happen before electron or web page are launched | |
| await trezorEnvSetup( | |
| testInfo, | |
| startEmulator, | |
| setupEmulator, | |
| emulatorStartConf, | |
| emulatorSetupConf, | |
| ); | |
| if (isDesktopProject(testInfo)) { | |
| const suite = await electronSetup(testInfo, locale, colorScheme); | |
| await use(suite.window); | |
| await electronTeardown(suite, testInfo); | |
| } else { | |
| const browserContext = await browser.newContext(); | |
| const page = await webSetup(browserContext); | |
| await use(page); | |
| await page.close(); | |
| await browserContext.close(); | |
| } | |
| await TrezorUserEnvLinkProxy.logTestDetails( | |
| ` - - - FINISHING TEST ${testInfo.titlePath.join(' - ')}`, | |
| ); | |
| }, | |
| exceptionLogger: [ | |
| async ({ page }, use) => { | |
| const errors: Error[] = []; | |
| page.on('pageerror', error => { | |
| errors.push(error); | |
| }); | |
| await use(); | |
| if (errors.length > 0) { | |
| throw new Error( | |
| `There was a JS exception during test run. | |
| \n${errors.map(error => `${error.message}\n${error.stack}`).join('\n-----\n')}`, | |
| ); | |
| } | |
| }, | |
| { auto: true }, | |
| ], | |
| }); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 101-101: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 105-105: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
There was a problem hiding this comment.
Yeah, I already tried that in past. Causes an error
First argument must use the object destructuring pattern: _
at ../support/testExtends/suiteBaseFixture.ts:95
93 | // Depending on the project type (desktop or web) it will launch the appropriate environment
94 | // and provide the necessary page object which is either electron window or web page
> 95 | const suiteBaseTest = base.extend<suiteBaseFixture>({
| ^
96 | startEmulator: true,
97 | setupEmulator: true,
98 | emulatorStartConf: { model: 'T3T1', wipe: true },
First argument must use the object destructuring pattern: _
There was a problem hiding this comment.
@Vere-Grey Thanks for sharing that error message. You're right - Playwright's base.extend requires object destructuring pattern specifically, not a simple underscore.
In this case, we can suppress the linter warnings properly using empty object patterns. I see you already have /* eslint-disable-next-line no-empty-pattern */ comments, but they might need to be positioned correctly. Here's a solution:
- /* eslint-disable-next-line no-empty-pattern */
url: async ({}, use, testInfo) => {
+ // Empty destructuring required by Playwright
await use(getUrl(testInfo));
},
- /* eslint-disable-next-line no-empty-pattern */
trezorUserEnvLink: async ({}, use) => {
+ // Empty destructuring required by Playwright
await use(TrezorUserEnvLinkProxy);
},Alternatively, you can use a destructured parameter that's explicitly named but unused:
- url: async ({}, use, testInfo) => {
+ url: async ({ /* unused */ }, use, testInfo) => {
await use(getUrl(testInfo));
},
- trezorUserEnvLink: async ({}, use) => {
+ trezorUserEnvLink: async ({ /* unused */ }, use) => {
await use(TrezorUserEnvLinkProxy);
},This clearly indicates the pattern is intentionally empty while satisfying both Playwright's requirements and the linter.
✏️ Learnings added
Learnt from: Vere-Grey
PR: trezor/trezor-suite#17287
File: packages/suite-desktop-core/e2e/support/testExtends/suiteBaseFixture.ts:92-164
Timestamp: 2025-02-27T17:02:27.605Z
Learning: When using Playwright's `base.extend` for fixtures, the first parameter must use object destructuring pattern (`{}`) even when no parameters are needed. Using a simple underscore (`_`) will cause an error.
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.
| onboardingPage: async ( | ||
| { page, analyticsPage, devicePrompt, emulatorStartConf }, | ||
| use, | ||
| testInfo, | ||
| ) => { | ||
| const onboardingPage = new OnboardingActions( | ||
| page, | ||
| analyticsPage, | ||
| devicePrompt, | ||
| emulatorStartConf.model, | ||
| testInfo, | ||
| ); | ||
| await use(onboardingPage); | ||
| await use(new WalletActions(page)); | ||
| }, | ||
| onboardingPage: async ({ page, emulatorStartConf }, use, testInfo) => { | ||
| await use(new OnboardingActions(page, emulatorStartConf.model, testInfo)); | ||
| }, |
There was a problem hiding this comment.
I am not entirely sure I like this. Some of the fixtures are not stateless and one could assume, that all of them are singletons. I know we have used initialization within the fixtures in the past as well, but I am not 100% sure that it is desired.
We can definitely use this approach, because I don't know what is right/wrong, I am just pointing it out as something to consider.
Motivation: Cleaner and more easily readable core part of our tests - fixtures
Untangle page fixture and trezor env setup. It was a working mess before :)
Split it to suiteBase and pageObjects + mocks
fixture file slimmed down
metadataProviderMock.stop() moved to fixture teardown