sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

test: containerized Chrome end-to-end harness that drives the real popup #181

New Issue
Closed
opened 2026-08-09 16:10:05 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-09 16:10:05 +02:00
Collaborator
Copy Link

Supersedes the "no browser available" premise of #173, which was wrong. A
containerized Chrome can load the unpacked MV3 build and drive the popup; this
has been measured, not assumed. See
#173 (comment) for the raw
output.

The single most important fact: the probe caught #150 - a live crash on main -
while make check was green on the same tree. This harness exists to close
exactly that gap.

Definition of done

  1. script/test-e2e exists and is the entrypoint, with make test-e2e as a
    thin shim, per the scripts-to-rule-them-all layout already used here.
  2. It builds dist/chrome/ and runs the suite inside the pinned container. It
    must NOT be part of script/test or make check - REPO_POLICIES caps
    make test at 20 seconds and browser tests will not fit.
  3. The suite drives the real popup in a real browser, not jsdom, and covers
    at minimum:
    • the popup loads and reaches #view-welcome with no uncaught page errors
    • full wallet creation through the UI (#btn-welcome-add →
      #btn-generate-phrase → password → #btn-add-wallet-confirm)
      lands on #view-main
    • the Add Token screen opens (this is #150 and it currently FAILS)
    • the transaction detail screen renders for an ERC-20 transfer (this is
      #151 and it currently FAILS)
  4. Any uncaught pageerror or console.error fails the run. This is the
    mechanism that caught #150 and it is the point of the whole exercise. A
    documented allowlist is acceptable only for the libsodium CSP error tracked
    separately, and each entry must name its tracking issue.
  5. The harness must be demonstrated to fail. The PR description must show
    the suite failing against unfixed main for #150 and #151, with the actual
    captured error text, and passing once they are fixed. A green run alone is
    not acceptable evidence - this repo has already produced two vacuous checks
    that passed on things they could not detect, and a harness nobody has seen
    fail is worth nothing.
  6. README documents make test-e2e, its container requirement, and that it is
    deliberately outside make check.

Implementation requirements

  • Pin the image by digest, as REPO_POLICIES requires of every external
    reference, with a comment giving the human-readable tag above it:

    # mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09
mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2

  • Add playwright-core as a devDependency pinned through yarn.lock. Use
    playwright-core, not playwright - the browsers already ship inside
    the image and a second download is pure waste. Its version must match the
    image's Playwright version (1.56.0); a mismatch is a real failure mode, so
    note the coupling in a comment next to the digest.

  • Launch with channel: "chromium". The default headless: true uses the
    headless shell, which silently refuses to load extensions - it does not
    error, the service worker simply never appears. This cost real debugging time;
    put it in a comment so the next person does not repeat it.

  • Discover the extension id from the service worker URL rather than hardcoding
    it; it is derived from the unpacked path and will not be stable.

  • Intercept all network at the browser level. The probe saw live outbound
    calls returning 401. The suite must be deterministic and must not depend on
    Blockscout, CoinDesk, the RPC endpoint or the phishing list being reachable.
    Route interception also supplies the stubbed ERC-20 transfer that #151 needs.

  • Fail loudly if the container cannot be started, rather than skipping and
    reporting success. A skip that looks like a pass is the failure mode this
    issue exists to prevent.

  • Do not wire this into the Gitea workflow in this PR. Docker-in-docker in CI
    is a separate question and lumping it in here would stall a change that is
    useful the moment it lands locally.

Out of scope

  • Firefox. Playwright's public API does not install extensions in Firefox, and
    #153's premise means Chrome cannot verify it. Tracked separately.
  • Any actual fix for #150 or #151 beyond what is needed to demonstrate the
    harness catching them. Land the harness with those tests present and failing
    if that is cleaner, or coordinate with the fixes - but the captured failure
    output must appear in the PR either way.
Supersedes the "no browser available" premise of #173, which was wrong. A containerized Chrome can load the unpacked MV3 build and drive the popup; this has been measured, not assumed. See https://git.eeqj.de/sneak/AutistMask/issues/173#issuecomment-49609 for the raw output. The single most important fact: the probe caught #150 - a live crash on `main` - while `make check` was green on the same tree. This harness exists to close exactly that gap. ## Definition of done 1. `script/test-e2e` exists and is the entrypoint, with `make test-e2e` as a thin shim, per the scripts-to-rule-them-all layout already used here. 2. It builds `dist/chrome/` and runs the suite inside the pinned container. It must NOT be part of `script/test` or `make check` - REPO_POLICIES caps `make test` at 20 seconds and browser tests will not fit. 3. The suite drives the **real popup in a real browser**, not jsdom, and covers at minimum: - the popup loads and reaches `#view-welcome` with no uncaught page errors - full wallet creation through the UI (`#btn-welcome-add` → `#btn-generate-phrase` → password → `#btn-add-wallet-confirm`) lands on `#view-main` - the Add Token screen opens (this is #150 and it currently FAILS) - the transaction detail screen renders for an ERC-20 transfer (this is #151 and it currently FAILS) 4. **Any uncaught `pageerror` or `console.error` fails the run.** This is the mechanism that caught #150 and it is the point of the whole exercise. A documented allowlist is acceptable only for the libsodium CSP error tracked separately, and each entry must name its tracking issue. 5. **The harness must be demonstrated to fail.** The PR description must show the suite failing against unfixed `main` for #150 and #151, with the actual captured error text, and passing once they are fixed. A green run alone is not acceptable evidence - this repo has already produced two vacuous checks that passed on things they could not detect, and a harness nobody has seen fail is worth nothing. 6. README documents `make test-e2e`, its container requirement, and that it is deliberately outside `make check`. ## Implementation requirements - **Pin the image by digest**, as REPO_POLICIES requires of every external reference, with a comment giving the human-readable tag above it: ``` # mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09 mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2 ``` - Add `playwright-core` as a devDependency pinned through `yarn.lock`. Use `playwright-core`, **not** `playwright` - the browsers already ship inside the image and a second download is pure waste. Its version must match the image's Playwright version (1.56.0); a mismatch is a real failure mode, so note the coupling in a comment next to the digest. - Launch with `channel: "chromium"`. The default `headless: true` uses the headless shell, which **silently refuses to load extensions** - it does not error, the service worker simply never appears. This cost real debugging time; put it in a comment so the next person does not repeat it. - Discover the extension id from the service worker URL rather than hardcoding it; it is derived from the unpacked path and will not be stable. - **Intercept all network at the browser level.** The probe saw live outbound calls returning `401`. The suite must be deterministic and must not depend on Blockscout, CoinDesk, the RPC endpoint or the phishing list being reachable. Route interception also supplies the stubbed ERC-20 transfer that #151 needs. - Fail loudly if the container cannot be started, rather than skipping and reporting success. A skip that looks like a pass is the failure mode this issue exists to prevent. - Do not wire this into the Gitea workflow in this PR. Docker-in-docker in CI is a separate question and lumping it in here would stall a change that is useful the moment it lands locally. ## Out of scope - Firefox. Playwright's public API does not install extensions in Firefox, and #153's premise means Chrome cannot verify it. Tracked separately. - Any actual fix for #150 or #151 beyond what is needed to demonstrate the harness catching them. Land the harness with those tests present and failing if that is cleaner, or coordinate with the fixes - but the captured failure output must appear in the PR either way.
clawbot added this to the 1.0.0 milestone 2026-08-09 16:10:05 +02:00
clawbot commented 2026-08-09 16:10:47 +02:00
Author
Collaborator
Copy Link

Sequencing decision, resolving the tension in the "Out of scope" section above.

The DoD requires the suite to be shown failing and then passing, but a PR whose
suite is red cannot land. Rather than merge a knowingly-red check or water the
tests down to something that passes vacuously, this PR carries the two fixes
the harness caught: #150 and #151 are both a single missing require import
each, and they are the direct evidence the harness works.

So the unit is: harness, plus the two one-line fixes it found, with the PR body
showing the captured failure output from before the fixes.

That keeps it commit-sized, it lands green, and the failure evidence is
preserved in the PR record where a reviewer can check it rather than having to
take it on trust.

#182 (the libsodium CSP page error) is explicitly NOT in this PR - it needs a
real decision about the extension CSP and it would swamp this change. It gets
the one allowlist entry permitted by DoD item 4, naming #182.

Sequencing decision, resolving the tension in the "Out of scope" section above. The DoD requires the suite to be shown failing and then passing, but a PR whose suite is red cannot land. Rather than merge a knowingly-red check or water the tests down to something that passes vacuously, **this PR carries the two fixes the harness caught**: #150 and #151 are both a single missing `require` import each, and they are the direct evidence the harness works. So the unit is: harness, plus the two one-line fixes it found, with the PR body showing the captured failure output from before the fixes. That keeps it commit-sized, it lands green, and the failure evidence is preserved in the PR record where a reviewer can check it rather than having to take it on trust. #182 (the libsodium CSP page error) is explicitly NOT in this PR - it needs a real decision about the extension CSP and it would swamp this change. It gets the one allowlist entry permitted by DoD item 4, naming #182.
clawbot commented 2026-08-09 16:14:07 +02:00
Author
Collaborator
Copy Link

Implementation plan

Taking the sequencing decision in the comment above: one unit = harness + the
two one-line import fixes it catches (#150, #151), with the pre-fix failure
output captured verbatim in the PR body.

Harness layout

  • tests/e2e/run.js — a plain Node runner (deliberately not jest: jest's
    default testMatch would drag this into script/test, and the 20 second cap
    in REPO_POLICIES.md is real). Files are named *.e2e.js / run.js so no
    jest pattern can ever pick them up.

  • script/test-e2e — builds dist/chrome/, then runs the runner inside the
    pinned container. make test-e2e is a thin shim. Neither script/test nor
    script/check will reference it.

  • Image pinned by digest with the tag and date in a comment above it, plus a
    note that playwright-core must stay version-matched to the image:

    # mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09
mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2

  • playwright-core@1.56.0 as an exact devDependency through yarn.lock. Not
    playwright — the browsers already live at /ms-playwright in the image.

  • Launch via chromium.launchPersistentContext with channel: "chromium",
    --load-extension / --disable-extensions-except / --no-sandbox, with a
    comment recording that the default headless shell silently refuses to load
    extensions (no error, the service worker just never appears).

  • Extension id read from new URL(ctx.serviceWorkers()[0].url()).host, with
    waitForEvent("serviceworker") as the fallback. Never hardcoded.

  • Missing/unstartable container is a hard failure with a nonzero exit, never a
    skip.

Determinism

ctx.route() intercepts every http/https request at the browser level and
fulfils it locally; anything not explicitly stubbed is failed loudly and
recorded, so a new outbound call cannot silently reintroduce flakiness. Stubs
cover the Blockscout v2 endpoints (/addresses/:a/transactions,
/token-transfers, /token-balances), the JSON-RPC endpoint (batched and
single, eth_getBalance plus a zero-word eth_call so ethers' ENS reverse
lookup resolves to "no resolver" rather than throwing — a throw would emit a
console.error and fail the run on its own), the CoinDesk price tick, and the
phishing blocklist. The probe saw live calls returning 401; after this, nothing
leaves the container.

Coverage

  1. popup loads, reaches #view-welcome, no uncaught errors
  2. wallet creation through the real UI lands on #view-main
  3. Add Token opens from #view-address (this is #150)
  4. transaction detail renders for a stubbed ERC-20 transfer, including the
    token-contract row (this is #151)

Every test asserts against a shared error collector wired to pageerror and
console.error on every page and service worker. One allowlist entry only, for
the libsodium WASM CSP error, naming #182 in a comment; #182 itself is out of
scope here.

The stubbed transfer is built to survive filterTransactions — an unknown
symbol (no spoof match) with holders_count above the 1000 threshold, since
hideLowHolderTokens defaults on. Otherwise the row would be filtered out and
the test would pass vacuously, which is exactly the failure mode this issue
exists to prevent.

Fixes carried

  • src/popup/views/addToken.js:1 — add showView to the destructure.
  • src/popup/views/transactionDetail.js:4-18 — add addressDotHtml. Not
    renderAddressHtml: that helper hardcodes etherscanAddressUrl
    (/address/...), while the token-contract row needs the token-specific
    /token/... link from #136. Swapping it in would regress that link, so the
    lower-level helper is the right call here; I will say so in the PR.

Order of work

Land the harness first and capture the two real failures against the unfixed
tree, then apply the fixes and capture the green run. Both outputs go in the PR
body verbatim. README.md gets make test-e2e, its container requirement, and
why it is deliberately outside make check. TODO.md updated in the same
commit. No Gitea workflow change in this PR.

## Implementation plan Taking the sequencing decision in the comment above: one unit = harness + the two one-line import fixes it catches (#150, #151), with the pre-fix failure output captured verbatim in the PR body. ### Harness layout - `tests/e2e/run.js` — a plain Node runner (deliberately not jest: jest's default `testMatch` would drag this into `script/test`, and the 20 second cap in `REPO_POLICIES.md` is real). Files are named `*.e2e.js` / `run.js` so no jest pattern can ever pick them up. - `script/test-e2e` — builds `dist/chrome/`, then runs the runner inside the pinned container. `make test-e2e` is a thin shim. Neither `script/test` nor `script/check` will reference it. - Image pinned by digest with the tag and date in a comment above it, plus a note that `playwright-core` must stay version-matched to the image: ``` # mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09 mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2 ``` - `playwright-core@1.56.0` as an exact devDependency through `yarn.lock`. Not `playwright` — the browsers already live at `/ms-playwright` in the image. - Launch via `chromium.launchPersistentContext` with `channel: "chromium"`, `--load-extension` / `--disable-extensions-except` / `--no-sandbox`, with a comment recording that the default headless shell silently refuses to load extensions (no error, the service worker just never appears). - Extension id read from `new URL(ctx.serviceWorkers()[0].url()).host`, with `waitForEvent("serviceworker")` as the fallback. Never hardcoded. - Missing/unstartable container is a hard failure with a nonzero exit, never a skip. ### Determinism `ctx.route()` intercepts every `http`/`https` request at the browser level and fulfils it locally; anything not explicitly stubbed is failed loudly and recorded, so a new outbound call cannot silently reintroduce flakiness. Stubs cover the Blockscout v2 endpoints (`/addresses/:a/transactions`, `/token-transfers`, `/token-balances`), the JSON-RPC endpoint (batched and single, `eth_getBalance` plus a zero-word `eth_call` so ethers' ENS reverse lookup resolves to "no resolver" rather than throwing — a throw would emit a `console.error` and fail the run on its own), the CoinDesk price tick, and the phishing blocklist. The probe saw live calls returning 401; after this, nothing leaves the container. ### Coverage 1. popup loads, reaches `#view-welcome`, no uncaught errors 2. wallet creation through the real UI lands on `#view-main` 3. Add Token opens from `#view-address` (this is #150) 4. transaction detail renders for a stubbed ERC-20 transfer, including the token-contract row (this is #151) Every test asserts against a shared error collector wired to `pageerror` and `console.error` on every page and service worker. One allowlist entry only, for the libsodium WASM CSP error, naming #182 in a comment; #182 itself is out of scope here. The stubbed transfer is built to survive `filterTransactions` — an unknown symbol (no spoof match) with `holders_count` above the 1000 threshold, since `hideLowHolderTokens` defaults on. Otherwise the row would be filtered out and the test would pass vacuously, which is exactly the failure mode this issue exists to prevent. ### Fixes carried - `src/popup/views/addToken.js:1` — add `showView` to the destructure. - `src/popup/views/transactionDetail.js:4-18` — add `addressDotHtml`. Not `renderAddressHtml`: that helper hardcodes `etherscanAddressUrl` (`/address/...`), while the token-contract row needs the token-specific `/token/...` link from #136. Swapping it in would regress that link, so the lower-level helper is the right call here; I will say so in the PR. ### Order of work Land the harness first and capture the two real failures against the unfixed tree, then apply the fixes and capture the green run. Both outputs go in the PR body verbatim. `README.md` gets `make test-e2e`, its container requirement, and why it is deliberately outside `make check`. `TODO.md` updated in the same commit. No Gitea workflow change in this PR.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#181
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?