test: drive ConfirmTx in the e2e suite, gate assertion included (closes #238) #258

clawbot · 2026-08-12T10:53:55+02:00

clawbot commented

2026-08-12 10:53:55 +02:00

Closes #238.

src/popup/views/confirmTx.js had no unit tests and make test-e2e never
reached the confirmation screen, so the screen that decides what gets signed
had no automated coverage of its own behaviour. The arithmetic underneath is
well covered in src/shared/txValidation.js; the gap was the wiring.

The load-bearing assertion

The confirmation screen quotes the fee estimate (gasLimit * gasPrice) and
gates on the fee reserve (gasLimit * maxFeePerGas). Reading the quoted
number instead is
#154 in full, and until now
that was correct by reading only. The fixture stubs the two far apart
(estimate 0.002121 ETH, reserve 0.004221 ETH) and sizes the refused sends
into the gap between them, so an estimate-reading gate accepts what a
reserve-reading gate refuses.

Demonstrated, not asserted by inspection. With feeWei = gasCostWei changed to
feeWei = estimateWei in estimateGas() — the mutant the review of
#197 described — the suite
goes red:

# gate probe: balance=1.0 ETH amount=0.997879 estimate=0.002121 ETH reserve=0.004221 ETH sendDisabled=false amountFeeError=false
not ok 17 - ConfirmTx gates on the fee RESERVE, not the displayed estimate (#238)
  Send is ENABLED for a transfer the fee RESERVE does not cover — the spend gate is reading the displayed estimate, which is issue #154
ok 18 - ConfirmTx refuses a send that exceeds the balance outright (#238)
ok 19 - ConfirmTx refuses to send when the fee estimate fails, with its own message (#238)
# confirm-tx ERC-20 view height: 1070.5625px
ok 20 - ConfirmTx drives the ERC-20 path from pending to funded (#238)
ok 21 - ConfirmTx refuses an ERC-20 send that exceeds the token balance (#238)
# erc-20 gate probe: ethBalance=0.002121 estimate=0.002121 ETH reserve=0.004221 ETH sendDisabled=false gasError=false
not ok 22 - ConfirmTx gates the ERC-20 fee on the RESERVE, with the ERC-20 message (#238)
  Send is ENABLED for an ERC-20 transfer whose fee RESERVE exceeds the ETH balance — the spend gate is reading the displayed estimate (#154)
ok 23 - ConfirmTx reports a failed ERC-20 estimate as unknown, not as a fee problem (#238)
# 21/23 tests passed
# FAILED

Both paths die, through different balances and different messages. The mutation
was reverted; git diff origin/next touches no file under src/.

What the harness gained

tests/e2e/network.js, in the established read-at-request-time style:

ethBalanceWei — hex wei answered to eth_getBalance, default zero, which
is what every pre-existing test expects.
seedTokenBalance — the ERC-20 holding that puts a token in the send
screen's dropdown, the only route to the ERC-20 confirmation path.
eth_getBlockByNumber with a baseFeePerGas. Without it ethers has no
maxFeePerGas, the reserve and the estimate collapse into one number, and
the gate tests would pass while asserting nothing. That trap is written down
next to the constants.
decimals() for the stub token on eth_call (every other call still answers
a zero word, so the ENS reverse lookup is untouched). A zero there makes
parseUnits() reject a fractional amount and the ERC-20 estimate fails for
the wrong reason.
failGasEstimate — a node-side refusal for eth_estimateGas.
holdGasEstimate — holds the estimate open so the pending state is observed
rather than raced. A hold that outlives its bound is reported like any other
harness fault.

ErrorCollector.expect() in tests/e2e/harness.js: a test that drives a
failure path provokes the console.error the code is supposed to emit, which
the harness fails a run on. A declaration consumes exactly one matching record,
and a declaration nothing matched fails its test just as an undeclared error
does — so it cannot be used as a mute. Both halves were verified by running
against a deliberately wrong pattern (console.error: [AutistMask] gas estimation failed: ... reported as an unexpected error) and by declaring an
expectation nothing emits (expected browser error(s) that never arrived: ...).

Coverage added

Nine tests, over both transaction types:

Send disabled while the estimate is pending, with no error message shown.
Send enabled for a comfortably funded send; the fee block quotes
~0.002121 ETH and up to 0.004221 ETH reserved separately.
Send disabled with the distinct fee-unknown message when the estimate fails,
and the over-budget message NOT shown.
Send disabled for a send past the balance outright, exact message asserted.
Send disabled for a send the reserve does not cover (both paths, above).
ERC-20 over token balance, exact message asserted.
View height constant across pending -> landed and pending -> failed, per
path: 1022.5625px for the ETH view and 1070.5625px for the ERC-20 view,
printed on every run.

TODO.md and the End-to-End Tests section of README.md updated in the same
commit.

Verification

make check: green. 19 suites, 416 tests, prettier --check clean.
make test-e2e: green, 23/23 (was 14/14 on next).
Both re-run after rebasing onto next at bd4bdca; the only conflict was
TODO.md, resolved keeping both entries.

Note for the reviewer: script/lint in this repo is prettier --check run on
the host, not in a container — there is no containerised lint target to invoke
yet. That is #152, untouched
here.

Not touched, per the issue: the empty-array POST guard
(#187), TRAILING_WATCH_MS,
and the service-worker interception canary.

Closes [#238](https://git.eeqj.de/sneak/AutistMask/issues/238). `src/popup/views/confirmTx.js` had no unit tests and `make test-e2e` never reached the confirmation screen, so the screen that decides what gets signed had no automated coverage of its own behaviour. The arithmetic underneath is well covered in `src/shared/txValidation.js`; the gap was the wiring. ## The load-bearing assertion The confirmation screen quotes the fee **estimate** (`gasLimit * gasPrice`) and gates on the fee **reserve** (`gasLimit * maxFeePerGas`). Reading the quoted number instead is [#154](https://git.eeqj.de/sneak/AutistMask/issues/154) in full, and until now that was correct by reading only. The fixture stubs the two far apart (estimate `0.002121 ETH`, reserve `0.004221 ETH`) and sizes the refused sends into the gap between them, so an estimate-reading gate accepts what a reserve-reading gate refuses. Demonstrated, not asserted by inspection. With `feeWei = gasCostWei` changed to `feeWei = estimateWei` in `estimateGas()` — the mutant the review of [#197](https://git.eeqj.de/sneak/AutistMask/pulls/197) described — the suite goes red: ``` # gate probe: balance=1.0 ETH amount=0.997879 estimate=0.002121 ETH reserve=0.004221 ETH sendDisabled=false amountFeeError=false not ok 17 - ConfirmTx gates on the fee RESERVE, not the displayed estimate (#238) Send is ENABLED for a transfer the fee RESERVE does not cover — the spend gate is reading the displayed estimate, which is issue #154 ok 18 - ConfirmTx refuses a send that exceeds the balance outright (#238) ok 19 - ConfirmTx refuses to send when the fee estimate fails, with its own message (#238) # confirm-tx ERC-20 view height: 1070.5625px ok 20 - ConfirmTx drives the ERC-20 path from pending to funded (#238) ok 21 - ConfirmTx refuses an ERC-20 send that exceeds the token balance (#238) # erc-20 gate probe: ethBalance=0.002121 estimate=0.002121 ETH reserve=0.004221 ETH sendDisabled=false gasError=false not ok 22 - ConfirmTx gates the ERC-20 fee on the RESERVE, with the ERC-20 message (#238) Send is ENABLED for an ERC-20 transfer whose fee RESERVE exceeds the ETH balance — the spend gate is reading the displayed estimate (#154) ok 23 - ConfirmTx reports a failed ERC-20 estimate as unknown, not as a fee problem (#238) # 21/23 tests passed # FAILED ``` Both paths die, through different balances and different messages. The mutation was reverted; `git diff origin/next` touches no file under `src/`. ## What the harness gained `tests/e2e/network.js`, in the established read-at-request-time style: - `ethBalanceWei` — hex wei answered to `eth_getBalance`, default zero, which is what every pre-existing test expects. - `seedTokenBalance` — the ERC-20 holding that puts a token in the send screen's dropdown, the only route to the ERC-20 confirmation path. - `eth_getBlockByNumber` with a `baseFeePerGas`. Without it ethers has no `maxFeePerGas`, the reserve and the estimate collapse into one number, and the gate tests would pass while asserting nothing. That trap is written down next to the constants. - `decimals()` for the stub token on `eth_call` (every other call still answers a zero word, so the ENS reverse lookup is untouched). A zero there makes `parseUnits()` reject a fractional amount and the ERC-20 estimate fails for the wrong reason. - `failGasEstimate` — a node-side refusal for `eth_estimateGas`. - `holdGasEstimate` — holds the estimate open so the pending state is observed rather than raced. A hold that outlives its bound is reported like any other harness fault. `ErrorCollector.expect()` in `tests/e2e/harness.js`: a test that drives a failure path provokes the `console.error` the code is supposed to emit, which the harness fails a run on. A declaration consumes exactly one matching record, and a declaration nothing matched fails its test just as an undeclared error does — so it cannot be used as a mute. Both halves were verified by running against a deliberately wrong pattern (`console.error: [AutistMask] gas estimation failed: ...` reported as an unexpected error) and by declaring an expectation nothing emits (`expected browser error(s) that never arrived: ...`). ## Coverage added Nine tests, over both transaction types: - Send disabled while the estimate is pending, with no error message shown. - Send enabled for a comfortably funded send; the fee block quotes `~0.002121 ETH` and `up to 0.004221 ETH reserved` separately. - Send disabled with the distinct fee-unknown message when the estimate fails, and the over-budget message NOT shown. - Send disabled for a send past the balance outright, exact message asserted. - Send disabled for a send the reserve does not cover (both paths, above). - ERC-20 over token balance, exact message asserted. - View height constant across pending -> landed and pending -> failed, per path: `1022.5625px` for the ETH view and `1070.5625px` for the ERC-20 view, printed on every run. `TODO.md` and the End-to-End Tests section of `README.md` updated in the same commit. ## Verification - `make check`: green. 19 suites, 416 tests, `prettier --check` clean. - `make test-e2e`: green, 23/23 (was 14/14 on `next`). - Both re-run after rebasing onto `next` at `bd4bdca`; the only conflict was `TODO.md`, resolved keeping both entries. Note for the reviewer: `script/lint` in this repo is `prettier --check` run on the host, not in a container — there is no containerised lint target to invoke yet. That is [#152](https://git.eeqj.de/sneak/AutistMask/issues/152), untouched here. Not touched, per the issue: the empty-array POST guard ([#187](https://git.eeqj.de/sneak/AutistMask/issues/187)), `TRAILING_WATCH_MS`, and the service-worker interception canary.

clawbot added 1 commit 2026-08-12 10:53:55 +02:00

test: drive ConfirmTx in the e2e suite, gate assertion included (closes #238 )

check / check (push) Successful in 32s

Details

7e5d7cdcde

The screen that decides what gets signed had no automated coverage of its
own behaviour: no unit tests, and the e2e suite never reached it. The
arithmetic underneath is well covered in src/shared/txValidation.js; the
gap was the wiring — which number reaches the gate, when the gate re-runs,
what the fee block renders, and whether Send is enabled.

The confirmation screen quotes the fee ESTIMATE (gasLimit * gasPrice) and
gates on the fee RESERVE (gasLimit * maxFeePerGas). Reading the quoted
number instead was issue #154, and until now that was correct by reading
only — a mutant swapping the two passed the whole suite. It no longer
does: two of the new tests fail on it, one per transaction type.

The harness gains a funded-balance fixture to make any of this reachable.
tests/e2e/network.js now serves a configurable ETH balance, an ERC-20
holding, a latest block with a baseFeePerGas (without which ethers has no
maxFeePerGas and the reserve and the estimate collapse into one number),
and decimals() for the stub token. It can also refuse a gas estimate, and
hold one open so the pending state can be observed rather than raced.

Nine tests, over both the native ETH and the ERC-20 path: Send disabled
while the estimate is pending, enabled once it lands, the fee block
quoting both numbers, the distinct message for an estimate that failed,
refusal for a send past the balance, refusal for a send the reserve does
not cover, and the view height constant across every one of those
transitions.

Driving a failure path means provoking the console.error the code is
supposed to emit, which the harness fails a run on. ErrorCollector gains
expect(): it consumes exactly one matching record, and a declaration
nothing matched fails its test just as an undeclared error does, so it
cannot be used to silence anything. Both halves of that were verified by
running the suite against a deliberately wrong pattern.

clawbot self-assigned this 2026-08-12 10:53:59 +02:00

clawbot added the needs-review label 2026-08-12 10:54:04 +02:00

clawbot referenced this pull request

2026-08-12 10:54:16 +02:00

test: ConfirmTx has no automated coverage at all — the screen that decides what gets signed #238

clawbot commented

2026-08-12 11:05:15 +02:00

FAIL — needs-rebase. One blocking finding; the change itself is sound and should not be touched.

Not fast-forwardable onto current origin/next. Branch head 7e5d7cd forks at bd4bdca; next is now afe6dda, two commits ahead (#221 at 23712b5, #155 at afe6dda). git merge-tree --write-tree origin/next 7e5d7cd exits 1 with CONFLICT (content): Merge conflict in TODO.md — both sides insert a bullet at the top of # Completed Steps. The PR body's rebase onto bd4bdca predates both. Acceptable: rebase onto current origin/next, resolve TODO.md keeping all three bullets (this one plus the #221 and #155 entries — the #221 entry does not exist at this branch's merge-base and must not be dropped), re-run both suites, force-push. No other change wanted.

Everything else verified and passing, independently reproduced on a clean clone: make check 19 suites / 416 tests, prettier clean; make test-e2e 23/23 executed; every DoD item real; scope clean (no file under src/); commit hygiene, authorship and terminology clean.

The gate assertion is not just reproducible, it is broad. Five independent mutations to src/popup/views/confirmTx.js, each reverted:

feeWei = gasCostWei to feeWei = estimateWei — 17 and 22 die, exactly as reported.
feeWei = 0n (gate ignores the fee) — 17 and 22 die.
feeReserveWei to feeEstimateWei (reserve collapsed onto estimate) — 16, 17, 20, 22 die; test 17's "this send is not in the gap" guard fires, so it refuses to pass vacuously when the fixture spread disappears.
estimate and reserve swapped in the fee block only, gate left correct — 16, 17, 20 die.
setVisible() collapsing space with display:none instead of reserving it, i.e. the layout-shift class of #252 — 16, 17, 19, 20, 22, 23 die on the height assertions across both paths and all three transitions. The height is genuinely asserted, not merely printed. Incidentally the pending height under that mutant is 874.5625px, matching the figure measured by hand in the review of #197.

ErrorCollector.expect() cannot mask a genuine error. Four probes: a second console.error matching the declared pattern in the same code path still fails its test (consumption is exactly one); an unrelated genuine console.error injected into the declaring test's own path still fails it; an expectation nothing matches fails its test (expected browser error(s) that never arrived); and an expectation declared in test 18 does not survive into test 19 — unmatchedExpectations() is called unconditionally after every test in tests/e2e/run.js:1079 and clears the list, so there is no forward leak. ALLOWED_ERRORS, TRAILING_WATCH_MS and the service-worker interception canary are untouched.

Disclosures, none blocking. The fixture widens what the RPC stub answers without reporting: eth_getBlockByNumber now returns the latest block for any block parameter, and eth_getCode is newly stubbed — previously both would have been reported as unstubbed methods. Fixture-shape choices, no outbound traffic escapes either way. The empty-array POST hole (#187) is unchanged, neither fixed nor worsened. Tracker CI status ignored per #220; host-only script/lint not held against this PR per #152.

FAIL — `needs-rebase`. One blocking finding; the change itself is sound and should not be touched. **Not fast-forwardable onto current `origin/next`.** Branch head `7e5d7cd` forks at `bd4bdca`; `next` is now `afe6dda`, two commits ahead ([#221](https://git.eeqj.de/sneak/AutistMask/issues/221) at `23712b5`, [#155](https://git.eeqj.de/sneak/AutistMask/issues/155) at `afe6dda`). `git merge-tree --write-tree origin/next 7e5d7cd` exits 1 with `CONFLICT (content): Merge conflict in TODO.md` — both sides insert a bullet at the top of `# Completed Steps`. The PR body's rebase onto `bd4bdca` predates both. Acceptable: rebase onto current `origin/next`, resolve `TODO.md` keeping all three bullets (this one plus the [#221](https://git.eeqj.de/sneak/AutistMask/issues/221) and [#155](https://git.eeqj.de/sneak/AutistMask/issues/155) entries — the [#221](https://git.eeqj.de/sneak/AutistMask/issues/221) entry does not exist at this branch's merge-base and must not be dropped), re-run both suites, force-push. No other change wanted. Everything else verified and passing, independently reproduced on a clean clone: `make check` 19 suites / 416 tests, prettier clean; `make test-e2e` 23/23 executed; every DoD item real; scope clean (no file under `src/`); commit hygiene, authorship and terminology clean. The gate assertion is not just reproducible, it is broad. Five independent mutations to `src/popup/views/confirmTx.js`, each reverted: - `feeWei = gasCostWei` to `feeWei = estimateWei` — 17 and 22 die, exactly as reported. - `feeWei = 0n` (gate ignores the fee) — 17 and 22 die. - `feeReserveWei` to `feeEstimateWei` (reserve collapsed onto estimate) — 16, 17, 20, 22 die; test 17's "this send is not in the gap" guard fires, so it refuses to pass vacuously when the fixture spread disappears. - estimate and reserve swapped in the fee block only, gate left correct — 16, 17, 20 die. - `setVisible()` collapsing space with `display:none` instead of reserving it, i.e. the layout-shift class of [#252](https://git.eeqj.de/sneak/AutistMask/issues/252) — 16, 17, 19, 20, 22, 23 die on the height assertions across both paths and all three transitions. The height is genuinely asserted, not merely printed. Incidentally the pending height under that mutant is 874.5625px, matching the figure measured by hand in the review of [#197](https://git.eeqj.de/sneak/AutistMask/pulls/197). `ErrorCollector.expect()` cannot mask a genuine error. Four probes: a second `console.error` matching the declared pattern in the same code path still fails its test (consumption is exactly one); an unrelated genuine `console.error` injected into the declaring test's own path still fails it; an expectation nothing matches fails its test (`expected browser error(s) that never arrived`); and an expectation declared in test 18 does not survive into test 19 — `unmatchedExpectations()` is called unconditionally after every test in `tests/e2e/run.js:1079` and clears the list, so there is no forward leak. `ALLOWED_ERRORS`, `TRAILING_WATCH_MS` and the service-worker interception canary are untouched. Disclosures, none blocking. The fixture widens what the RPC stub answers without reporting: `eth_getBlockByNumber` now returns the `latest` block for any block parameter, and `eth_getCode` is newly stubbed — previously both would have been reported as unstubbed methods. Fixture-shape choices, no outbound traffic escapes either way. The empty-array POST hole ([#187](https://git.eeqj.de/sneak/AutistMask/issues/187)) is unchanged, neither fixed nor worsened. Tracker CI status ignored per [#220](https://git.eeqj.de/sneak/AutistMask/issues/220); host-only `script/lint` not held against this PR per [#152](https://git.eeqj.de/sneak/AutistMask/issues/152).

clawbot added needs-rebase and removed needs-review labels 2026-08-12 11:06:03 +02:00

clawbot force-pushed test/issue-238-confirmtx-coverage from 7e5d7cdcde to ecfddeb327

2026-08-12 11:28:22 +02:00

Compare

clawbot force-pushed test/issue-238-confirmtx-coverage from ecfddeb327 to 5092cf162a

2026-08-12 11:33:58 +02:00

Compare

clawbot merged commit 5af89a1b63 into next

2026-08-12 11:35:21 +02:00

clawbot deleted branch test/issue-238-confirmtx-coverage

2026-08-12 11:35:22 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#258