refactor: one shared extension-API module, and drive the dApp flows on Firefox (closes #153)
All checks were successful
check / check (push) Successful in 28s
All checks were successful
check / check (push) Successful in 28s
Every call site that touched `browser.*` or `chrome.*` now goes through `src/shared/browserApi.js`, the only file in the tree that names either. It exposes lazily-resolved namespace handles for events and synchronous methods, and promise-returning wrappers for everything that is callback-shaped on Chrome. Callers await; `runtime.lastError` is gone, folded into the rejection the wrapper produces on the Chrome path. The Firefox suite gains the four dApp round trips the issue's definition of done asks for — `eth_requestAccounts`, `personal_sign`, `eth_sendTransaction`, and a closed approval window rejecting with EIP-1193 4001 — driven through the real content script, background page and approval windows. `--network none` was thought to rule that out because it leaves no `http://` origin to inject into; loopback survives it, so the page and a JSON-RPC node are served from 127.0.0.1 inside the container and the run still reaches nothing but itself. That harness refutes the premise it was built to verify. On Firefox 153.0.3, `browser.*` honours a trailing Chrome-style callback and does populate `runtime.lastError`, both measured directly, and all four flows pass against the unconverted code. So this is a uniformity and coverage change, not a repair of a broken target; the PR records the measurement in full. One real defect is fixed on the way past: the window id written back into a pending approval after `windows.create()` was unguarded, so an approval settled during the open — an address switch will do it — dereferenced a deleted entry.
This commit is contained in:
78
README.md
78
README.md
@@ -243,10 +243,18 @@ and this suite exists because exactly that class of bug shipped twice.
|
||||
|
||||
`make test-e2e-firefox` builds `dist/firefox/` and drives the **real popup in a
|
||||
real Firefox**, installed as an unpacked MV2 temporary add-on via geckodriver.
|
||||
It covers popup load, wallet creation through the UI, and the Add Token screen.
|
||||
The suite lives in `tests/e2e/firefox/` and has **no npm dependencies at all**:
|
||||
it is a small WebDriver client built on global `fetch` and `child_process`
|
||||
against geckodriver's HTTP API.
|
||||
It covers popup load, wallet creation through the UI, the Add Token screen, and
|
||||
the four dApp round trips — `eth_requestAccounts`, `personal_sign`,
|
||||
`eth_sendTransaction`, and a closed approval window rejecting with EIP-1193 4001
|
||||
— driven through the real content script, background page and approval windows.
|
||||
|
||||
The suite lives in `tests/e2e/firefox/`. Its WebDriver client (`driver.js`) has
|
||||
**no npm dependencies at all**: it is built on global `fetch` and
|
||||
`child_process` against geckodriver's HTTP API. The dApp fixture (`dapp.js`) and
|
||||
the assertions do use `ethers`, and have to — a signature is recovered in the
|
||||
runner rather than believed from the extension, and the stub node has to answer
|
||||
`eth_sendRawTransaction` with the hash `ethers` computes for the artifact it
|
||||
sent, or `provider.broadcastTransaction()` refuses the answer.
|
||||
|
||||
Unlike the Chrome suite it builds its own container image rather than pulling a
|
||||
published one, because no published image carries both a pinned Firefox and a
|
||||
@@ -268,20 +276,30 @@ because BiDi's `browsingContext.navigate` refuses `moz-extension://` outright.
|
||||
**Any uncaught error from a `moz-extension://` source fails the run**, including
|
||||
errors from the background page, which the suite never navigates to: a `throw`
|
||||
at the top of `src/background/index.js` kills the background page and fails
|
||||
step 1. Content-script errors should arrive by the same route, but this suite
|
||||
does not exercise it and does not claim it — with `--network none` there is no
|
||||
`http://` page for a content script to be injected into. Errors from add-on
|
||||
install and background startup are folded into step 1 rather than discarded.
|
||||
Errors are read from the privileged `nsIConsoleService` in Marionette's chrome
|
||||
context and filtered to non-warning entries whose `sourceName` is the extension
|
||||
origin. That mechanism is not a stylistic choice. WebDriver BiDi's
|
||||
`log.entryAdded` delivers **nothing** for extension pages: on a plain `http://`
|
||||
page it reports uncaught errors with stack traces, and on the `moz-extension://`
|
||||
popup it reports zero events, because Firefox's remote agent excludes extension
|
||||
browsing contexts from BiDi observation. Any harness built on Playwright-BiDi or
|
||||
Puppeteer-BiDi would therefore see nothing and report success, which is exactly
|
||||
the vacuous check this repo has already shipped twice. Do not migrate this suite
|
||||
to BiDi.
|
||||
step 1. Content scripts **are** exercised now — the dApp steps drive a page
|
||||
served from loopback, which survives `--network none` — but the _capture_ of a
|
||||
content-script error by this route is still unproven: no probe has forced a
|
||||
throw inside one and watched it fail the run, so it remains an expectation
|
||||
rather than a demonstrated fact. Errors from add-on install and background
|
||||
startup are folded into step 1 rather than discarded.
|
||||
|
||||
One error is tolerated rather than fatal, listed in `ALLOWED_ERRORS` in
|
||||
`tests/e2e/firefox/run.js` with the issue that will delete it, and printed on
|
||||
every occurrence so the concession stays visible in the run output. It is
|
||||
Firefox reporting the site-approval popup's unawaited `sendMessage` settling
|
||||
after `window.close()` unloaded the context — the same teardown ordering as
|
||||
[#275](https://git.eeqj.de/sneak/AutistMask/issues/275), and unsuppressable from
|
||||
the calling code, because `BaseContext.wrapPromise` reports it whether or not a
|
||||
handler is attached. Errors are read from the privileged `nsIConsoleService` in
|
||||
Marionette's chrome context and filtered to non-warning entries whose
|
||||
`sourceName` is the extension origin. That mechanism is not a stylistic choice.
|
||||
WebDriver BiDi's `log.entryAdded` delivers **nothing** for extension pages: on a
|
||||
plain `http://` page it reports uncaught errors with stack traces, and on the
|
||||
`moz-extension://` popup it reports zero events, because Firefox's remote agent
|
||||
excludes extension browsing contexts from BiDi observation. Any harness built on
|
||||
Playwright-BiDi or Puppeteer-BiDi would therefore see nothing and report
|
||||
success, which is exactly the vacuous check this repo has already shipped twice.
|
||||
Do not migrate this suite to BiDi.
|
||||
|
||||
Two limits are worth knowing, both real differences from the Chrome suite:
|
||||
|
||||
@@ -305,17 +323,19 @@ Two limits are worth knowing, both real differences from the Chrome suite:
|
||||
but a step that logs heavily could evict unread errors. What poll-based costs
|
||||
is location, not coverage: an error cannot be placed within a step the way the
|
||||
Chrome suite's `pageerror` events place it.
|
||||
- **Nothing is stubbed, which inverts the coverage of network-dependent code.**
|
||||
There is no fixture layer; the container runs with `--network none` instead,
|
||||
so the run is offline and deterministic and no request can escape. The
|
||||
extension swallows its own fetch failures, so the flows are unaffected — but
|
||||
every network call fails, so only the _failure_ branches of code that depends
|
||||
on one are ever executed. A `ReferenceError` in the success path of
|
||||
`renderTransactions`, or of price or balance rendering, passes this suite
|
||||
green. The offline run is also weaker than the Chrome suite's interception: it
|
||||
proves nothing got out, but it cannot report which requests were attempted.
|
||||
Closing that gap needs a fixture layer, deliberately out of scope for this
|
||||
harness.
|
||||
- **Almost nothing is stubbed, which inverts the coverage of network-dependent
|
||||
code.** The container still runs with `--network none`, so the run is offline
|
||||
and no request can escape. The one thing it can reach is the loopback fixture
|
||||
in `tests/e2e/firefox/dapp.js`, which serves the dApp page and a JSON-RPC node
|
||||
and which the extension's `rpcUrl` is pointed at for the dApp steps; a
|
||||
JSON-RPC method that fixture does not model fails the run rather than
|
||||
answering `null`. Everything else — Blockscout, the price feed, the phishing
|
||||
blocklist — has no fixture and simply fails, and the extension swallows its
|
||||
own fetch failures, so only the _failure_ branches of that code are ever
|
||||
executed. A `ReferenceError` in the success path of `renderTransactions`, or
|
||||
of price rendering, passes this suite green. The offline run is also weaker
|
||||
than the Chrome suite's interception for those calls: it proves nothing got
|
||||
out, but it cannot report which requests were attempted.
|
||||
|
||||
Neither `make test-e2e` nor `make test-e2e-firefox` is part of `make check` or
|
||||
`make test`. `REPO_POLICIES.md` caps `make test` at 20 seconds and a browser
|
||||
|
||||
Reference in New Issue
Block a user