test: e2e hygiene — a failing case leaves routeOpts and the popup screen dirty, cascading into unrelated failures #318

New Issue

2026-08-20T12:31:41+02:00

clawbot commented

2026-08-20 12:31:41 +02:00

Found by the independent review of #314 (#314 (comment)). Test-harness hygiene, no product defect. Matters because it makes a real failure harder to read, and #287 already means red runs have to be interpreted rather than believed.

Three findings, all in the Chrome suite:

Falsy || swallows a zero override. ethCallResult() in tests/e2e/network.js uses ||, so a decimals override of 0 is silently ignored and the default is served instead. A zero-decimal token is a legitimate case and cannot currently be fixtured.
A failing case restores neither routeOpts nor the popup screen. Demonstrated during the review: a deliberate mutation produced 2 real failures but 9 red tests, with cases 48-54 collateral damage from the leaked state. The real signal was buried in noise.
Case 43 fails as a 60-second timeout before its strongest assertion runs. The broadcast-count assertion — the one that actually proves nothing was sent — sits behind a screen wait, so a genuine regression costs a minute and reports the weaker fact.

Also noted, no action needed unless it bothers you: mismatchMessage in src/shared/transferAmount.js is an unused export, and transferAmountUnits("-1", 6, 6n) returns a negative bigint. The latter is unreachable — the send screen validates the amount before it gets there — and carries no funds risk.

Definition of done

ethCallResult() distinguishes an absent override from a zero one, and a zero-decimal token can be fixtured.
A case that throws restores routeOpts and returns the popup to a known screen — a finally, not cleanup on the success path.
Case 43 asserts the broadcast count before waiting on a screen, so a regression reports the strong fact fast.
make test-e2e still passes 54/54, and a deliberately mutated case 43 fails in under 10 seconds with only itself red. State the mutation used and the observed result.
make check green.

Found by the independent review of https://git.eeqj.de/sneak/AutistMask/pulls/314 (https://git.eeqj.de/sneak/AutistMask/pulls/314#issuecomment-67464). Test-harness hygiene, no product defect. Matters because it makes a real failure harder to read, and https://git.eeqj.de/sneak/AutistMask/issues/287 already means red runs have to be interpreted rather than believed. Three findings, all in the Chrome suite: 1. **Falsy `||` swallows a zero override.** `ethCallResult()` in `tests/e2e/network.js` uses `||`, so a `decimals` override of `0` is silently ignored and the default is served instead. A zero-decimal token is a legitimate case and cannot currently be fixtured. 2. **A failing case restores neither `routeOpts` nor the popup screen.** Demonstrated during the review: a deliberate mutation produced 2 real failures but 9 red tests, with cases 48-54 collateral damage from the leaked state. The real signal was buried in noise. 3. **Case 43 fails as a 60-second timeout before its strongest assertion runs.** The broadcast-count assertion — the one that actually proves nothing was sent — sits behind a screen wait, so a genuine regression costs a minute and reports the weaker fact. Also noted, no action needed unless it bothers you: `mismatchMessage` in `src/shared/transferAmount.js` is an unused export, and `transferAmountUnits("-1", 6, 6n)` returns a negative bigint. The latter is unreachable — the send screen validates the amount before it gets there — and carries no funds risk. ## Definition of done - [ ] `ethCallResult()` distinguishes an absent override from a zero one, and a zero-decimal token can be fixtured. - [ ] A case that throws restores `routeOpts` and returns the popup to a known screen — a `finally`, not cleanup on the success path. - [ ] Case 43 asserts the broadcast count before waiting on a screen, so a regression reports the strong fact fast. - [ ] `make test-e2e` still passes 54/54, and a deliberately mutated case 43 fails in under 10 seconds with **only itself** red. State the mutation used and the observed result. - [ ] `make check` green.

clawbot referenced this issue

2026-08-20 12:56:35 +02:00

fix: show the true token amount on the approval screen, or none at all (closes #306) #321

clawbot referenced this issue

2026-08-20 12:57:50 +02:00

test: guard decimals:0 against the falsy trap, and de-duplicate toDecimals #325

clawbot referenced this issue

2026-08-20 14:26:18 +02:00

pre-1.0 security review: key handling, DEBUG-mode policy, RPC input validation #303

Sign in to join this conversation.