-
Notifications
You must be signed in to change notification settings - Fork 2
test(web-wallet): E2E follow-ups from #177 (PR 1 — light bundle) #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| /** | ||
| * `SNAP_TIMEOUT_MS` is a production-facing build-time knob (webpack DefinePlugin). It defaults to | ||
| * empty, which MUST resolve to the shipped 10s. These tests pin that default so a future refactor | ||
| * cannot silently change production behavior. The module reads the env at import time, so each | ||
| * case re-imports against a fresh module registry. | ||
| */ | ||
| async function loadSnapTimeouts() { | ||
| vi.resetModules(); | ||
| return (await import('../timeouts')).SNAP_TIMEOUTS; | ||
| } | ||
|
|
||
| describe('SNAP_TIMEOUTS default (SNAP_TIMEOUT_MS)', () => { | ||
| afterEach(() => { | ||
| vi.unstubAllEnvs(); | ||
| }); | ||
|
|
||
| it('resolves every timeout to the shipped 10s when SNAP_TIMEOUT_MS is empty', async () => { | ||
| vi.stubEnv('SNAP_TIMEOUT_MS', ''); | ||
| const snapTimeouts = await loadSnapTimeouts(); | ||
| expect(Object.values(snapTimeouts).length).toBeGreaterThan(0); | ||
| for (const value of Object.values(snapTimeouts)) { | ||
| expect(value).toBe(10000); | ||
| } | ||
| }); | ||
|
|
||
| it('applies a valid positive override to every timeout', async () => { | ||
| vi.stubEnv('SNAP_TIMEOUT_MS', '60000'); | ||
| const snapTimeouts = await loadSnapTimeouts(); | ||
| for (const value of Object.values(snapTimeouts)) { | ||
| expect(value).toBe(60000); | ||
| } | ||
| }); | ||
|
|
||
| it.each(['0', '-5', 'abc'])('falls back to 10s for invalid SNAP_TIMEOUT_MS=%s', async (raw) => { | ||
| vi.stubEnv('SNAP_TIMEOUT_MS', raw); | ||
| const snapTimeouts = await loadSnapTimeouts(); | ||
| for (const value of Object.values(snapTimeouts)) { | ||
| expect(value).toBe(10000); | ||
| } | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,13 @@ | ||
| import type { BrowserContext, Locator, Page } from '@playwright/test'; | ||
| import { MetaMask, MetaMaskText, Srp, Unlock } from './selectors'; | ||
| import { SPLIT_WINDOWS } from './windows'; | ||
| import { TIMEOUTS } from './timeouts'; | ||
| import { TIMEOUTS } from '../timeouts'; | ||
|
|
||
| const DEFAULT_PASSWORD = 'Test1234!Test'; | ||
|
|
||
| /** This Flask build's SRP length. Kept explicit so the reveal read is bounded and checkable. */ | ||
| const SEED_WORD_COUNT = 12; | ||
|
|
||
| /** | ||
| * Drives the real MetaMask Flask extension: onboarding a fresh wallet, and approving the | ||
| * Snap connect/install/dialog flows. | ||
|
|
@@ -158,20 +161,38 @@ export class MetaMaskDriver { | |
| private async revealAndSaveSeed(): Promise<string[]> { | ||
| const page = await this.openHome(); | ||
| await page.getByTestId(MetaMask.onboarding.recoveryReveal).click(); | ||
|
|
||
| // Chips populate asynchronously; on a slow render a one-shot read can catch an empty chip | ||
| // and turn a transient into a hard failure. Poll the read (bounded by TIMEOUTS.seedReveal) | ||
| // until every word is present, THEN keep the fail-fast throw for a genuinely missing word. | ||
| const deadline = Date.now() + TIMEOUTS.seedReveal; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion(non-blocking): The deadline is only checked after Passing a short per-read |
||
| let words: string[] = []; | ||
| for (;;) { | ||
| words = await this.readSeedWords(page); | ||
| if (words.length === SEED_WORD_COUNT && words.every((word) => word)) break; | ||
| if (Date.now() >= deadline) { | ||
| await this.dumpScreen('recovery-phrase-missing-word'); | ||
| throw new Error( | ||
| `Failed to read all ${SEED_WORD_COUNT} MetaMask recovery words within ` + | ||
| `${TIMEOUTS.seedReveal}ms (read: ${JSON.stringify(words)}).`, | ||
| ); | ||
| } | ||
| await page.waitForTimeout(300); | ||
| } | ||
|
|
||
| this.seedPhrase = words.join(' '); | ||
| await page.getByTestId(MetaMask.onboarding.recoveryContinue).click(); | ||
| await page.waitForTimeout(1200); | ||
| return words; | ||
| } | ||
|
|
||
| /** Reads the SRP chips once; empty/missing chips come back as empty strings (letters only). */ | ||
| private async readSeedWords(page: Page): Promise<string[]> { | ||
| const words: string[] = []; | ||
| for (let i = 0; i < 12; i++) { | ||
| for (let i = 0; i < SEED_WORD_COUNT; i++) { | ||
| const raw = (await page.getByTestId(Srp.chip(i)).textContent().catch(() => '')) || ''; | ||
| words.push(raw.replace(/[^a-z]/gi, '')); | ||
| } | ||
| // An empty word breaks the quiz later as an opaque timeout — fail at the source. | ||
| if (words.some((word) => !word)) { | ||
| await this.dumpScreen('recovery-phrase-missing-word'); | ||
| throw new Error('Failed to read all MetaMask recovery words.'); | ||
| } | ||
| this.seedPhrase = words.join(' '); | ||
| await page.getByTestId(MetaMask.onboarding.recoveryContinue).click(); | ||
| await page.waitForTimeout(1200); | ||
| return words; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,45 @@ | ||
| import { execFileSync } from 'node:child_process'; | ||
| import { existsSync, mkdirSync, readdirSync, statSync } from 'node:fs'; | ||
| import { existsSync, mkdirSync, readdirSync, readFileSync, statSync } from 'node:fs'; | ||
| import { dirname, join, resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { EXPECTED_FLASK_VERSION } from './selectors'; | ||
|
|
||
| const here = dirname(fileURLToPath(import.meta.url)); | ||
| const CACHE_DIR = resolve(here, '..', '.cache'); | ||
|
|
||
| /** | ||
| * Guards against silent MetaMask drift: selectors are pinned to EXPECTED_FLASK_VERSION, so a | ||
| * different build (e.g. a silent auto-update, or a stale `.cache`) rots selectors into baffling | ||
| * mid-journey failures. Fail loudly and actionably at resolve time instead. | ||
| * `E2E_ALLOW_FLASK_MISMATCH=1` downgrades to a warning for someone intentionally validating a new | ||
| * version. | ||
| */ | ||
| function assertFlaskVersion(manifestDir: string): void { | ||
| let actual = ''; | ||
| try { | ||
| const manifest = JSON.parse(readFileSync(join(manifestDir, 'manifest.json'), 'utf8')) as { | ||
| version?: string; | ||
| }; | ||
| actual = manifest.version ?? ''; | ||
| } catch { | ||
| return; // an unreadable/absent manifest is already handled by findManifestDir; don't double-fail | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: an unparseable manifest silently bypasses the version guard
|
||
| } | ||
| // The manifest reports a 4-part build (e.g. "13.31.0.150"); the pin is the 3-part prefix. | ||
| if (actual === EXPECTED_FLASK_VERSION || actual.startsWith(`${EXPECTED_FLASK_VERSION}.`)) return; | ||
|
|
||
| const message = | ||
| `MetaMask Flask version mismatch: driver/selectors.ts is pinned to ` + | ||
| `${EXPECTED_FLASK_VERSION}, but the loaded build is "${actual || 'unknown'}". ` + | ||
| `Selectors are version-sensitive — re-verify them (see "Upgrading the pinned MetaMask ` + | ||
| `version" in docs/qa-automation-strategy.md), then bump EXPECTED_FLASK_VERSION. ` + | ||
| `Set E2E_ALLOW_FLASK_MISMATCH=1 to proceed anyway.`; | ||
| if (process.env.E2E_ALLOW_FLASK_MISMATCH === '1') { | ||
| console.warn(`[flask] WARNING: ${message}`); | ||
| return; | ||
| } | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| /** | ||
| * Resolves a path to an *unpacked* MetaMask Flask build to load as a Chromium extension. | ||
| * | ||
|
|
@@ -30,6 +64,7 @@ export async function resolveFlaskPath(): Promise<string> { | |
| `Point it at an unpacked MetaMask Flask build directory.`, | ||
| ); | ||
| } | ||
| assertFlaskVersion(found); | ||
| return found; | ||
| } | ||
|
|
||
|
|
@@ -47,7 +82,10 @@ export async function resolveFlaskPath(): Promise<string> { | |
|
|
||
| const dest = join(CACHE_DIR, `flask-${version}`); | ||
| const cached = existsSync(dest) ? findManifestDir(dest) : null; | ||
| if (cached) return cached; | ||
| if (cached) { | ||
| assertFlaskVersion(cached); | ||
| return cached; | ||
| } | ||
|
|
||
| mkdirSync(dest, { recursive: true }); | ||
| const assetUrl = await discoverFlaskAssetUrl(version); | ||
|
|
@@ -69,6 +107,7 @@ export async function resolveFlaskPath(): Promise<string> { | |
| if (!unpacked) { | ||
| throw new Error(`Downloaded Flask ${version} but no manifest.json found under ${dest}.`); | ||
| } | ||
| assertFlaskVersion(unpacked); | ||
| return unpacked; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,12 @@ | |
|
|
||
| **Read this before reading or writing any E2E test in `tests/e2e/`.** | ||
|
|
||
| > **This suite is on-demand and NON-GATING.** It is not run per-PR and does not block merges. | ||
| > A red run here means "a real-MetaMask journey didn't pass on this machine right now" — often | ||
| > a slow cold-start, not a broken product. Run it deliberately with `yarn e2e` (see | ||
| > `docs/qa-automation-strategy.md`); investigate failures, but never treat this suite as a | ||
| > required check. | ||
|
|
||
| These tests drive the **real MetaMask Flask extension** + the **Hathor Snap** against the | ||
| web-wallet's actual UI. The rules below are the architectural decisions the suite is built | ||
| on. They exist to prevent recurring confusion about *where each piece of logic belongs* — | ||
|
|
@@ -232,6 +238,16 @@ These bit us once each — check for them when a `getByRole`/`getByText` fails: | |
| `flex justify-between` row the label text and the toggle button are siblings, so from the | ||
| label go up two levels (`xpath=ancestor::div[2]`) before selecting the button. | ||
|
|
||
| ## Two timeout knobs — which to reach for | ||
|
|
||
| - **`E2E_TIMEOUT_SCALE`** (test side) multiplies every deadline in `tests/e2e/timeouts.ts` — | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion(non-blocking): file map still lists Pinned to this changed line; the concern is the Worth knowing a stale-path grep won't catch this; the map is an indented tree, so the path never appears as a literal string. |
||
| the MetaMask driver's waits *and* the page object's `TIMEOUTS.dapp` waits. Reach for it to | ||
| stretch the whole suite for a slow machine/CI (e.g. `E2E_TIMEOUT_SCALE=2`). | ||
| - **`SNAP_TIMEOUT_MS`** (dApp side) raises the web-wallet's own hard snap-RPC cap (default 10s; | ||
| see `src/constants/timeouts.ts`), injected at build time via webpack `DefinePlugin`. Reach | ||
| for it when a slow snap **cold-start** trips the dApp's internal RPC race (the e2e webServer | ||
| already sets `SNAP_TIMEOUT_MS=60000`). It does NOT affect Playwright waits. | ||
|
|
||
| ## File map | ||
|
|
||
| ```text | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: checklist step 4 points at a checksum that doesn't exist yet
Item 6 is explicitly out of scope for this PR and
driver/flask.tshas no checksum today, so anyone following this checklist right now stalls on a step with nothing to update. Worth marking it as pending until that PR lands.