test: the Settings view has no browser coverage, so a wrong element id takes the whole screen down undetected #229

New Issue

clawbot · 2026-08-11T15:09:37+02:00

clawbot commented

2026-08-11 15:09:37 +02:00

Nothing exercises the Settings view in a browser. The e2e suite never navigates there, and jest runs in the node environment with no DOM, so every $("settings-...") lookup in src/popup/views/settings.js is unverified at runtime.

Consequence: a single wrong element id makes init() throw and takes the entire Settings screen down — network selector, theme, spam filters, wallet management, the lot. make check passes, review passes on a static grep, and the user finds it.

This is precisely the undefined-identifier class that shipped as #150 and #151. Settings is the densest concentration of id lookups in the codebase and has the least coverage of any screen.

Raised while adding a new checkbox in #176, where the author correctly flagged that their static verification was weaker than a run.

Partly overlapping but NOT a substitute: #152 adds ESLint, which would catch a genuinely undefined identifier. It would not catch a string id that is valid JavaScript but does not match any element in index.html — which is the actual failure mode here.

Implementation requirements

Add e2e coverage that creates a wallet, navigates to Settings, and asserts the screen renders with no page error.
Assert the controls are present and functional, not merely that the view is visible: the four Token Spam Protection checkboxes render with their expected default states, toggling one persists across a popup reopen, and the network and theme selectors render.
Make a wrong id FAIL the suite loudly. Any uncaught page error while on Settings must fail the run — the harness already treats console.error as a failure, so confirm a thrown init() actually surfaces that way rather than being swallowed.
Consider a cheap general guard alongside the specific test: assert every $("...") id referenced by the view modules exists in src/popup/index.html. That catches the whole class rather than the ids one test happens to touch, and is a static check that could run in make check.

Definition of done

An e2e test reaches Settings, asserts the spam-protection checkboxes and the network and theme selectors render, and toggles one setting across a reopen.
Deliberately breaking one element id in settings.js fails the suite — demonstrated, with captured output.
A green run cannot mean the Settings assertions were skipped.
TODO.md updated in the same commit.
make check passes.

Nothing exercises the Settings view in a browser. The e2e suite never navigates there, and jest runs in the node environment with no DOM, so every `$("settings-...")` lookup in `src/popup/views/settings.js` is unverified at runtime. Consequence: a single wrong element id makes `init()` throw and takes the **entire Settings screen** down — network selector, theme, spam filters, wallet management, the lot. `make check` passes, review passes on a static grep, and the user finds it. This is precisely the undefined-identifier class that shipped as https://git.eeqj.de/sneak/AutistMask/issues/150 and https://git.eeqj.de/sneak/AutistMask/issues/151. Settings is the densest concentration of id lookups in the codebase and has the least coverage of any screen. Raised while adding a new checkbox in https://git.eeqj.de/sneak/AutistMask/issues/176, where the author correctly flagged that their static verification was weaker than a run. Partly overlapping but NOT a substitute: https://git.eeqj.de/sneak/AutistMask/issues/152 adds ESLint, which would catch a genuinely undefined identifier. It would not catch a string id that is valid JavaScript but does not match any element in `index.html` — which is the actual failure mode here. ## Implementation requirements - Add e2e coverage that creates a wallet, navigates to Settings, and asserts the screen renders with no page error. - Assert the controls are present and functional, not merely that the view is visible: the four Token Spam Protection checkboxes render with their expected default states, toggling one persists across a popup reopen, and the network and theme selectors render. - Make a wrong id FAIL the suite loudly. Any uncaught page error while on Settings must fail the run — the harness already treats `console.error` as a failure, so confirm a thrown `init()` actually surfaces that way rather than being swallowed. - Consider a cheap general guard alongside the specific test: assert every `$("...")` id referenced by the view modules exists in `src/popup/index.html`. That catches the whole class rather than the ids one test happens to touch, and is a static check that could run in `make check`. ## Definition of done - [ ] An e2e test reaches Settings, asserts the spam-protection checkboxes and the network and theme selectors render, and toggles one setting across a reopen. - [ ] Deliberately breaking one element id in `settings.js` fails the suite — demonstrated, with captured output. - [ ] A green run cannot mean the Settings assertions were skipped. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot added this to the 1.0.0 milestone 2026-08-11 15:09:37 +02:00

clawbot referenced this issue

2026-08-11 15:36:18 +02:00

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

clawbot commented

2026-08-14 06:03:16 +02:00

Plan.

e2e (Chrome suite, tests/e2e/run.js) — a new Settings section, additive, following the existing shape and reusing the openSettings() helper already there:

reach Settings from the created wallet, assert #view-settings renders;
assert the four Token Spam Protection checkboxes are present, are real input[type=checkbox], and default to checked (hideSpoofedSymbols, hideLowHolderTokens, hideFraudContracts, hideDustTransactions all default true in src/shared/state.js);
toggle one by clicking it, close the page, reopen the popup, and assert it comes back off — that exercises the change handler, saveState(), loadState() and the init() assignment, not just visibility;
assert #settings-network and #settings-theme are select elements whose options match NETWORKS / the three theme values and whose current value equals persisted state.

Placed after the address-removal section so it runs before the sections that own the fixture state, and it leaves the popup back on #view-main.

Skip-detection. Each Settings test records a coverage key in env; a final guard test asserts the recorded set equals the expected set exactly. An early return, a deleted test or a silently-swallowed body then fails the run rather than shrinking it quietly.

Thrown init(). settings.init(ctx) runs from the DOMContentLoaded handler in src/popup/index.js, so a wrong id throws inside an async function, i.e. as a promise rejection rather than a synchronous throw. I will verify empirically that that reaches the harness collector as a pageerror/console.error and fails the run, rather than assuming it — and if it is swallowed, the fix goes in the harness.

Demonstration. Break one id in src/popup/views/settings.js on the show() path and one on the init() path, capture both failing runs into the PR body, then revert.

Static guard. A jest test asserting every literal $("id") under src/popup/ has a matching id="..." in src/popup/index.html, so it runs in make check. Measured on next: 221 distinct referenced ids, 274 ids in the markup, 0 missing — no false positives today. Scoped to $("...") deliberately: literal document.getElementById("...") would drag in debug-banner, which is created at runtime and legitimately absent from the markup.

TODO.md in the same commit.

Plan. **e2e (Chrome suite, `tests/e2e/run.js`)** — a new Settings section, additive, following the existing shape and reusing the `openSettings()` helper already there: - reach Settings from the created wallet, assert `#view-settings` renders; - assert the four Token Spam Protection checkboxes are present, are real `input[type=checkbox]`, and default to checked (`hideSpoofedSymbols`, `hideLowHolderTokens`, `hideFraudContracts`, `hideDustTransactions` all default `true` in `src/shared/state.js`); - toggle one by clicking it, close the page, reopen the popup, and assert it comes back off — that exercises the `change` handler, `saveState()`, `loadState()` and the `init()` assignment, not just visibility; - assert `#settings-network` and `#settings-theme` are `select` elements whose options match `NETWORKS` / the three theme values and whose current value equals persisted state. Placed after the address-removal section so it runs before the sections that own the fixture state, and it leaves the popup back on `#view-main`. **Skip-detection.** Each Settings test records a coverage key in `env`; a final guard test asserts the recorded set equals the expected set exactly. An early return, a deleted test or a silently-swallowed body then fails the run rather than shrinking it quietly. **Thrown `init()`.** `settings.init(ctx)` runs from the `DOMContentLoaded` handler in `src/popup/index.js`, so a wrong id throws inside an async function, i.e. as a promise rejection rather than a synchronous throw. I will verify empirically that that reaches the harness collector as a `pageerror`/`console.error` and fails the run, rather than assuming it — and if it is swallowed, the fix goes in the harness. **Demonstration.** Break one id in `src/popup/views/settings.js` on the `show()` path and one on the `init()` path, capture both failing runs into the PR body, then revert. **Static guard.** A jest test asserting every literal `$("id")` under `src/popup/` has a matching `id="..."` in `src/popup/index.html`, so it runs in `make check`. Measured on `next`: 221 distinct referenced ids, 274 ids in the markup, 0 missing — no false positives today. Scoped to `$("...")` deliberately: literal `document.getElementById("...")` would drag in `debug-banner`, which is created at runtime and legitimately absent from the markup. `TODO.md` in the same commit.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#229