Skip to content

Commit 16206b0

Browse files
committed
atnipattern
1 parent 667c18a commit 16206b0

2 files changed

Lines changed: 154 additions & 0 deletions

File tree

domains/testing/skills/e2e-testing/repos/metamask-extension.md

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,84 @@ The following patterns are prohibited in test specs:
417417
await somePage.waitForLoadingToComplete();
418418
```
419419

420+
4. **Test-Step Helpers Belong in Page Objects or Flow Files, Not Spec Files**
421+
422+
Spec files must not define helper functions that perform test steps (UI interactions or multi-step navigation). Spec files should read as a sequence of high-level page object and flow calls. Inline step helpers hide test steps, cannot be reused across specs, and bypass the Page Object Model / Flow structure the framework mandates.
423+
424+
**Decision rule for where the logic belongs:**
425+
426+
- Touches a **single** page object → add a **method to that Page Object class**.
427+
- Orchestrates **more than one** page object → move it to a **Flow file** (`test/e2e/page-objects/flows/*.flow.ts`).
428+
429+
> This applies to helpers that perform **test steps** (UI/navigation/step logic). Small pure-data utilities are not the target.
430+
431+
The example below touches three page objects (`HeaderNavbar`, `SettingsPage`, `SyncAccountsSettingsPage`), so it belongs in a flow file.
432+
433+
❌ Incorrect — helper function performing test steps defined inside the `.spec.ts` file:
434+
435+
```typescript
436+
// qr-sync.spec.ts
437+
async function navigateToSyncAccountsSettings(
438+
driver: Driver,
439+
): Promise<SyncAccountsSettingsPage> {
440+
const headerNavbar = new HeaderNavbar(driver);
441+
await headerNavbar.openSettingsPage();
442+
443+
const settingsPage = new SettingsPage(driver);
444+
await settingsPage.checkPageIsLoaded();
445+
await settingsPage.goToSyncAccountsSettings();
446+
447+
const syncAccountsPage = new SyncAccountsSettingsPage(driver);
448+
await syncAccountsPage.checkPageIsLoaded();
449+
await syncAccountsPage.waitForQrCode();
450+
return syncAccountsPage;
451+
}
452+
453+
describe('QrSync', function () {
454+
it('syncs a single HD wallet to mobile', async function () {
455+
// ...
456+
const syncAccountsPage = await navigateToSyncAccountsSettings(driver);
457+
// ...
458+
});
459+
});
460+
```
461+
462+
✅ Correct — move it to a flow file because it spans multiple page objects:
463+
464+
```typescript
465+
// test/e2e/page-objects/flows/sync-accounts.flow.ts
466+
export async function navigateToSyncAccountsSettings(
467+
driver: Driver,
468+
): Promise<SyncAccountsSettingsPage> {
469+
const headerNavbar = new HeaderNavbar(driver);
470+
await headerNavbar.openSettingsPage();
471+
472+
const settingsPage = new SettingsPage(driver);
473+
await settingsPage.checkPageIsLoaded();
474+
await settingsPage.goToSyncAccountsSettings();
475+
476+
const syncAccountsPage = new SyncAccountsSettingsPage(driver);
477+
await syncAccountsPage.checkPageIsLoaded();
478+
await syncAccountsPage.waitForQrCode();
479+
return syncAccountsPage;
480+
}
481+
```
482+
483+
```typescript
484+
// qr-sync.spec.ts
485+
import { navigateToSyncAccountsSettings } from '../../page-objects/flows/sync-accounts.flow';
486+
487+
describe('QrSync', function () {
488+
it('syncs a single HD wallet to mobile', async function () {
489+
// ...
490+
const syncAccountsPage = await navigateToSyncAccountsSettings(driver);
491+
// ...
492+
});
493+
});
494+
```
495+
496+
If the logic had touched only **one** page object, the correct fix would instead be to add a method to that page object class rather than create a flow.
497+
420498
## Handling Flaky Tests
421499

422500
### Common Issues and Solutions
@@ -515,6 +593,7 @@ Before submitting E2E tests, ensure:
515593
- [ ] Page Object pattern used for all UI interactions
516594
- [ ] Element selectors defined in page objects, not in test specs
517595
- [ ] No hardcoded selectors in test files
596+
- [ ] No test-step/navigation helper functions defined in spec files — extract to a page object method (single page) or a flow file (multiple pages)
518597
- [ ] Proper TypeScript type annotations used for variables and method parameters
519598

520599
### Test Reliability

domains/testing/skills/e2e-testing/repos/metamask-mobile.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,80 @@ The following patterns are prohibited in test specs:
242242
await Assertions.expectElementToBeVisible(element);
243243
```
244244

245+
4. **Test-Step Helpers Belong in Page Objects or Flow Files, Not Spec Files**
246+
247+
Spec files must not define helper functions that perform test steps (UI interactions or multi-step navigation). Spec files should read as a sequence of high-level page object and flow calls. Inline step helpers hide test steps, cannot be reused across specs, and bypass the Page Object Model / Flow structure the framework mandates.
248+
249+
**Decision rule for where the logic belongs:**
250+
251+
- Touches a **single** page object → add a **method to that Page Object class**.
252+
- Orchestrates **more than one** page object → move it to a **Flow file** (`e2e/flows/*.flow.ts`, e.g. the existing `flows/wallet.flow.ts`).
253+
254+
> This applies to helpers that perform **test steps** (UI/navigation/step logic). Small pure-data utilities are not the target.
255+
256+
The example below touches three page objects (`TabBarComponent`, `SettingsView`, `SyncAccountsSettingsView`), so it belongs in a flow file.
257+
258+
❌ Incorrect — helper function performing test steps defined inside the `.spec.ts` file:
259+
260+
```typescript
261+
// qr-sync.spec.ts
262+
async function navigateToSyncAccountsSettings(): Promise<void> {
263+
await TabBarComponent.tapSettingButton();
264+
await SettingsView.tapSyncAccounts();
265+
await SyncAccountsSettingsView.expectScreenVisible();
266+
await SyncAccountsSettingsView.waitForQrCode();
267+
}
268+
269+
describe(SmokeE2E('QrSync'), () => {
270+
it('syncs a single HD wallet to mobile', async () => {
271+
await withFixtures(
272+
{ fixture: new FixtureBuilder().build(), restartDevice: true },
273+
async () => {
274+
await loginToApp();
275+
await navigateToSyncAccountsSettings();
276+
// ...
277+
},
278+
);
279+
});
280+
});
281+
```
282+
283+
✅ Correct — move it to a flow file because it spans multiple page objects:
284+
285+
```typescript
286+
// e2e/flows/sync-accounts.flow.ts
287+
import SettingsView from '../page-objects/Settings/SettingsView';
288+
import SyncAccountsSettingsView from '../page-objects/Settings/SyncAccountsSettingsView';
289+
import TabBarComponent from '../page-objects/wallet/TabBarComponent';
290+
291+
export async function navigateToSyncAccountsSettings(): Promise<void> {
292+
await TabBarComponent.tapSettingButton();
293+
await SettingsView.tapSyncAccounts();
294+
await SyncAccountsSettingsView.expectScreenVisible();
295+
await SyncAccountsSettingsView.waitForQrCode();
296+
}
297+
```
298+
299+
```typescript
300+
// qr-sync.spec.ts
301+
import { navigateToSyncAccountsSettings } from '../../flows/sync-accounts.flow';
302+
303+
describe(SmokeE2E('QrSync'), () => {
304+
it('syncs a single HD wallet to mobile', async () => {
305+
await withFixtures(
306+
{ fixture: new FixtureBuilder().build(), restartDevice: true },
307+
async () => {
308+
await loginToApp();
309+
await navigateToSyncAccountsSettings();
310+
// ...
311+
},
312+
);
313+
});
314+
});
315+
```
316+
317+
If the logic had touched only **one** page object, the correct fix would instead be to add a method to that page object class rather than create a flow.
318+
245319
## Handling Flaky Tests
246320

247321
### Common Issues and Solutions
@@ -302,6 +376,7 @@ Before submitting E2E tests, ensure:
302376
- [ ] All gestures have descriptive `description` parameters
303377
- [ ] Appropriate timeouts for operations (not magic numbers)
304378
- [ ] Page Object pattern used for complex interactions
379+
- [ ] No test-step/navigation helper functions defined in spec files — extract to a page object method (single page) or a flow file (multiple pages)
305380
- [ ] Element selectors defined once and reused
306381
- [ ] Framework configuration used appropriately
307382
- [ ] Error handling for expected failure scenarios

0 commit comments

Comments
 (0)