MetaMask / MetaMask/metamask-extension

[P1] Benchmark step timers are not a disjoint partition of the flow they measure

Open
#45,452 1 comment 0 reactions 0 assignees View on GitHub
INVALID-ISSUE-TEMPLATE Sev2-normal ta-ai-fixable ta-needs-engineer-escalation ta-triaged team-qa
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

**Parent Epic:** [#6944 Performance Quality Gates](https://github.com/MetaMask/MetaMask-planning/issues/6944) → [#7182 Benchmarks: Reliability](https://github.com/MetaMask/MetaMask-planning/issues/7182) → [#7181 Benchmark Harness Fixes](https://github.com/MetaMask/MetaMask-planning/issues/7181)
**File:** `test/e2e/benchmarks/flows/user-journey/onboarding-import-wallet.ts:184` (at `0350a018`)
**Size:** M | **Hours:** ~6-10

---

## Problem

The step timers are not a partition of the flow. `measure()` in [`timer-helper.ts:55-63`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/timer-helper.ts#L55-L63) starts and stops an independent `performance.now()` pair per step, and nothing constrains those pairs to be adjacent — the flows do real work between them.

Two consequences:

1. **Named steps exclude their own trigger.** `onboarding-new-wallet.ts` calls `completeOnboarding()` and `handleSidepanelPostOnboarding(driver)` at lines 150–152, *before* starting the timer named `doneButtonToAssetList` at line 156. The Done-button click is outside a metric named for it. `onboarding-import-wallet.ts` has the identical shape around `doneButtonToHomeScreen` (`completeOnboarding()` at 172, `handleSidepanelPostOnboarding(driver)` at 173, timer opens at 177).
2. **Adjacent labels trade a shared wait.** `doneButtonToHomeScreen` and `openAccountMenuToAccountListLoaded` are two labels either side of a floating boundary — a single ~5.6s wait is billed to whichever step it lands in.

`total` is the sum of the step timers rather than the elapsed run — [`runner.ts:242-244`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/runner.ts#L242-L244) reduces the per-run timer values into `runTotal` — so time in the gaps between steps is in no metric at all. How much time that is is what [#45462 fix step timers measuring non-disjoint adjacent spans](https://github.com/MetaMask/metamask-extension/pull/45462) (open) is written to make measurable.

`doneButtonToHomeScreen` was demoted out of `GATED_METRIC_VALUES` by [#45444 demote the three bimodal onboarding metrics out of the gate](https://github.com/MetaMask/metamask-extension/pull/45444) (merged 2026-08-28), so the gate no longer reads boundary placement as duration. The metric is still measured and still published, and it cannot be restored — its restore condition is [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266) — until the boundaries are disjoint.

### Observation

Per-iteration values from two `chrome-webpack-userJourneyOnboardingImport` job logs (2026-08-05, mocked population):

| run | `doneButtonToHomeScreen` | `openAccountMenuToAccountListLoaded` | sum |
|---|---|---|---|
| [31026037892](https://github.com/MetaMask/metamask-extension/actions/runs/31026037892) | 3.38, 3.48, 3.34, **9.72**, 3.19 | 7.87, 7.51, 7.88, **1.05**, 8.15 | 11.25, 10.99, 11.22, 10.77, 11.34 |
| [31042229025](https://github.com/MetaMask/metamask-extension/actions/runs/31042229025) | 3.35, 3.42, **8.94**, 3.24, **8.77** | 6.82, 6.87, **1.05**, 6.85, **1.04** | 10.17, 10.29, 9.99, 10.09, 9.81 |

Whenever one step is slow the other collapses to ~1.05s, and **the sum is flat** (sd ≈ 0.2s).

Across 30 sampled runs, within-run standard deviation:

| metric | median within-run sd |
|---|---:|
| `doneButtonToHomeScreen` | 2662ms |
| `openAccountMenuToAccountListLoaded` | 2674ms |
| `total` | **120ms** (< 400ms in 30/30 runs) |

Near-identical spread in the two components against a stable container is the signature of two variables trading a fixed quantity. Median ratio `sd(doneButton)/sd(total)` = 12.4×.

A second, related defect follows from the same cause: in **16 of 30** runs the two components **sum to more than `total`** (e.g. 8625 + 6519 = 15144 against a total of 10292). The gated steps are not a partition of the flow, so reasoning about `total` as the sum of its steps — which the threshold table implicitly does by carrying ceilings for both — is not valid.

### Mechanism

[`onboarding-import-wallet.ts:184`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/flows/user-journey/onboarding-import-wallet.ts#L184) closes `doneButtonToHomeScreen` on `waitForTokenToBeDisplayed('Solana', 120000)`, then `openAccountMenu()` opens the next step at line 191. When the token is already present, the wait returns immediately and the still-running work blocks the *following* step instead. Same work, different label. [`onboarding-new-wallet.ts:163`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/flows/user-journey/onboarding-new-wallet.ts#L163) is the same call with a 60000ms timeout, closing `doneButtonToAssetList`.

Distinct from — but sharing a root with — [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266), which covers making that wait deterministic. This ticket is about the step boundaries themselves: even with a deterministic wait, adjacent timers that can absorb each other's work are not measuring what their names claim.

---

## Solution

1. **Make boundaries deterministic.** A step should open and close on events that cannot reorder relative to each other, not on a wait whose completion races the boundary.
2. **Then choose per metric:** merge the two steps into one gated span, or keep them separate with boundaries that guarantee disjointness.
3. **Give the flow a `total` that can disagree with its steps, then assert against it.** The assertion as originally written here cannot fire: [`performance-tracker.ts`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/performance-tracker.ts#L63-L80) computes `metrics.total` by summing exactly the durations it pushes into `metrics.steps`, so the steps sum to `total` identically, always. There is no flow-level span to compare them against, which is why no coverage gap has ever been detectable. Measure the whole flow as its own span first, then `total` minus the sum of steps becomes a real number: the work the harness does not measure. Assert on that residual, and publish it rather than discarding it.
4. **Fail on a step that records nothing.** The same loop filters on `duration > 0`, so a step that produced no duration is dropped from both the step list and the sum without trace. A broken timer currently makes `total` smaller.

---

## Acceptance Criteria

- [ ] The onboarding flows' step timers partition the span they cover, or the unaccounted remainder is reported
- [ ] A step named for a UI action includes that action
- [ ] Sum of a flow's measured steps never exceeds its `total`; violation fails the run
- [ ] `doneButtonToHomeScreen` within-run sd falls to the order of `total`'s (~120ms)
- [ ] The same audit is applied to the other gated step timers, which have not been checked for this defect
- [x] `doneButtonToHomeScreen` out of `GATED_METRIC_VALUES` until the above lands — [#45444 demote the three bimodal metrics](https://github.com/MetaMask/metamask-extension/pull/45444), merged 2026-08-28

---

## Labels

`team-extension-platform`, `area-testSuite`

---

## Dependencies

**Related:** [#7202 deterministic render-complete waits for account menu and swap](https://github.com/MetaMask/MetaMask-planning/issues/7202) (the account menu is one half of this boundary), [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266), [#45205 restore benchmark gate signal](https://github.com/MetaMask/metamask-extension/issues/45205), [#45046 benchmark `quality-gate` red on every `main` commit since 2026-07-17](https://github.com/MetaMask/metamask-extension/issues/45046), [#45462 fix step timers measuring non-disjoint adjacent spans](https://github.com/MetaMask/metamask-extension/pull/45462)

Contributor guide

Open the contributing guide

Research direction

Start with test/e2e/benchmarks/flows/user-journey/onboarding-import-wallet.ts and onboarding-new-wallet.ts, then read utils/timer-helper.ts, runner.ts, and performance-tracker.ts. Trace the timer boundaries and flow total before running the onboarding benchmark checks. Done means the action-inclusive steps are disjoint or the residual is reported, invalid sums fail, and the other gated timers are audited.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
performance, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.