diff --git a/README.md b/README.md index 4d75b97..0e8756d 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,16 @@ intercepted at the browser level and served from fixtures in unrecognised outbound requests are reported as failures rather than silently allowed. +That reporting has one bound worth knowing. Observation ends when the browser +context is torn down, and nothing can watch traffic after that, so the run keeps +collecting for a fixed grace period after the last test returns +(`TRAILING_WATCH_MS` in `tests/e2e/run.js`, currently 1500ms) and then closes +the context. A request whose _first_ dispatch falls after that window is never +seen at all and cannot fail the run. In practice a request a test fires without +awaiting reaches the route handler about 10ms later, and anything on a repeating +timer gets observed on an earlier tick during the ~20s suite — but a one-shot +call deliberately deferred past the window will escape. + That interception covers the MV3 background service worker as well as the popup page, which it does not by default — `script/test-e2e` sets `PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1` for it. Because that flag is diff --git a/tests/e2e/harness.js b/tests/e2e/harness.js index 0a55a97..f51fe78 100644 --- a/tests/e2e/harness.js +++ b/tests/e2e/harness.js @@ -52,28 +52,22 @@ function isAllowed(text) { // // So there is no window left to fall outside of. take() is the only // reader and it always takes everything outstanding, so successive takes -// partition the entire record stream with no gaps. seal() closes the -// stream once the run is over and routes anything later straight to a -// callback rather than into a list nobody reads again. Attribution is -// therefore total by construction, and the runner turns every attributed -// record into a failure. +// partition the entire record stream with no gaps, and the runner turns +// every record it reads into a failure. +// +// Observation ends when the browser context is closed. Nothing records +// after that — the route handler and the console listeners are gone with +// the context — so there is no post-teardown phase to collect, and this +// class deliberately offers no mechanism pretending to cover one. class ErrorCollector { constructor() { this.entries = []; this.taken = 0; - this.onLate = null; } record(kind, text) { const line = kind + ": " + String(text).split("\n")[0]; if (isAllowed(line)) return; - if (this.onLate) { - // Sealed: no test and no suite phase is left to attribute - // this to, so hand it over now instead of accumulating it - // where nothing will look. - this.onLate(line); - return; - } this.entries.push(line); } @@ -84,11 +78,6 @@ class ErrorCollector { this.taken = this.entries.length; return out; } - - // Close the stream: later records go to onLate instead of the list. - seal(onLate) { - this.onLate = onLate; - } } function attachErrorListeners(ctx, errors) { diff --git a/tests/e2e/network.js b/tests/e2e/network.js index 413a45b..034e940 100644 --- a/tests/e2e/network.js +++ b/tests/e2e/network.js @@ -131,6 +131,23 @@ function handleRpc(route, postData, report) { } // ethers batches by default, so the body may be an array. const batch = Array.isArray(payload) ? payload : [payload]; + + // Anything that is not a JSON-RPC object, or a batch of them, is not + // RPC at all and must be reported like any other unrecognised + // outbound traffic rather than dereferenced. request.postData() + // returns null both for a bodyless POST and for a body Playwright + // cannot decode as UTF-8 (sendBeacon with a Blob, or any binary + // payload), so this is not an empty-string special case: it rejects + // every non-object payload, exactly as the catch above rejects every + // unparseable one. + if ( + payload === null || + typeof payload !== "object" || + !batch.every((req) => req !== null && typeof req === "object") + ) { + report("unstubbed request: POST " + route.request().url()); + return route.abort(); + } const replies = batch.map((req) => { const result = RPC_RESULTS[req.method]; if (result === undefined) { diff --git a/tests/e2e/run.js b/tests/e2e/run.js index f20f0a9..a771e3c 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -167,7 +167,13 @@ async function main() { // nowhere) // end of test k .. end of k+1 -> test k+1 // last test .. teardown -> the suite, via the trailing drain - // after the trailing drain -> seal(), which fails on the spot + // + // Those three phases cover the entire life of the browser context. + // There is no fourth: once the context is closed nothing can record, + // because the route handler and the console listeners died with it. + // Traffic that a test defers past the trailing drain is therefore + // never observed at all — a real limit of this design, stated in the + // README, and not one any post-teardown hook could close. // // Two green-but-vacuous runs on this harness were the same shape: a // record falling outside somebody's window and being dropped. First @@ -230,17 +236,6 @@ async function main() { // reported, and they fail the run. const trailing = session.errors.take(); - // From here the run is over and there is nothing left to attribute a - // record to, so stragglers fail immediately instead of piling up - // where nothing will read them. - let late = 0; - session.errors.seal((line) => { - late += 1; - console.log("# FAILED: browser error recorded after the run ended"); - console.log("# " + line); - process.exitCode = 1; - }); - console.log( "# " + (tests.length - failed) + "/" + tests.length + " tests passed", ); @@ -257,7 +252,7 @@ async function main() { } } - if (failed > 0 || trailing.length > 0 || late > 0) { + if (failed > 0 || trailing.length > 0) { console.log("# FAILED"); process.exitCode = 1; }