Browser runner: stall-based timeout, phase-aware page errors, afterEach on failure (0.2.19)

CHANGELOG.md

@@ -2,6 +2,14 @@
 
 All notable changes to this project will be documented in this file.
 
+## [0.2.19] - 2026-08-12
+
+### Fixed
+- **Browser timeout is stall-based, not an absolute deadline** (crank #375, firefox). `--timeout` means per-file on node/bun, but the browser runs the whole suite as one bundle - a single absolute deadline meant a 30-file suite (browser launch included) had to finish in one file's allowance, so large suites "timed out" at the default while making steady progress. Progress - the entry's ready flag, the runner starting, each completed test - now resets the clock; `timeout` ms with NO progress is a genuine hang. Crank's 602-test suite passes on firefox at the default timeout. (Measured while diagnosing: firefox's cost is ~8s of launch overhead, not bundle parsing - the 1.6MB bundle parses in under 600ms.)
+- **Uncaught page errors are phase-aware; mid-run unhandled rejections no longer fail the run.** 0.2.17 failed the run on ANY `pageerror`, but browsers don't agree on what reaches it - firefox surfaces unhandled rejections there, chromium/webkit don't - so suites that deliberately float rejections to test error propagation (crank's async generators) failed on firefox only, with a message ("tests registered after this error never ran") their own registration counts disproved. Uncaught errors are now captured in the page, where phase is known synchronously: before the runner starts, registration was genuinely cut short - red; during the run (module bodies are complete by ESM guarantee), nothing was lost - a yellow warning. Identical behavior on all three browsers.
+- **A bundle that throws during load fails in seconds, naming the error** - a module-body throw means the runner can never start, so the harness no longer waits out the full timeout in silence.
+- **Browser runner: a failing test no longer skips its own `afterEach`** (#25). Cleanup hooks now run in all cases (each individually guarded, the test's own error kept as the reported failure), matching node:test/bun:test. Skipping cleanup on failure compounds: one real failure left a `console.error` stub installed and crank's webkit run reported 23 failures for 1 bug, burying the real assertion text.
+
 ## [0.2.18] - 2026-08-12
 
 ### Fixed

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@b9g/libuild",
-  "version": "0.2.18",
+  "version": "0.2.19",
   "description": "Zero-config library builds",
   "main": "./dist/libuild.cjs",
   "bin": {

src/_test-browser.ts

@@ -268,22 +268,58 @@ export function expect(actual: unknown): Matchers {
 declare global {
   var __LIBUILD_TEST__: {
     ended: boolean;
+    started: boolean;
     failed: number;
     passed: number;
     skipped: number;
     errors: Array<{ name: string; error: string }>;
+    /** Uncaught errors BEFORE the runner started: registration was cut short */
+    loadErrors: string[];
+    /** Uncaught errors/unhandled rejections DURING the run: surfaced, not fatal */
+    runtimeErrors: string[];
   };
   var __LIBUILD_TEST_READY__: boolean | undefined;
 }
 
 globalThis.__LIBUILD_TEST__ = {
   ended: false,
+  started: false,
   failed: 0,
   passed: 0,
   skipped: 0,
   errors: [],
+  loadErrors: [],
+  runtimeErrors: [],
 };
 
+// Capture uncaught errors IN THE PAGE, where the current phase is known
+// synchronously - Playwright's `pageerror` can't tell load from run, and the
+// browsers don't even agree on what reaches it (firefox surfaces unhandled
+// rejections there; chromium/webkit don't). The phase is the whole meaning:
+//  - before the runner starts, an uncaught error means a test file's body
+//    aborted mid-registration and everything after it silently never
+//    registered - that MUST fail the run;
+//  - during the run, module bodies have all completed (the runner starts on
+//    the entry's ready flag), so nothing can have been lost - an unhandled
+//    rejection floating out of a test is reported, not fatal. Suites that
+//    deliberately exercise error propagation (crank's async generators)
+//    produce these while every assertion passes.
+function noteUncaught(kind: string, detail: unknown): void {
+  const message = `${kind}: ${
+    detail instanceof Error ? detail.message : String(detail ?? "unknown error")
+  }`;
+  const state = globalThis.__LIBUILD_TEST__;
+  (state.started ? state.runtimeErrors : state.loadErrors).push(message);
+}
+// globalThis, not window: this module has no DOM lib types, and in a browser
+// they are the same object.
+(globalThis as any).addEventListener("error", (event: any) => {
+  noteUncaught("uncaught error", event.error ?? event.message);
+});
+(globalThis as any).addEventListener("unhandledrejection", (event: any) => {
+  noteUncaught("unhandled rejection", event.reason);
+});
+
 type TestFn = () => void | Promise<void>;
 type HookFn = () => void | Promise<void>;
 
@@ -403,6 +439,10 @@ setTimeout(async () => {
     await new Promise((resolve) => setTimeout(resolve, 0));
   }
 
+  // Registration is complete; from here an uncaught error can't have eaten
+  // registrations (see noteUncaught).
+  globalThis.__LIBUILD_TEST__.started = true;
+
   const suiteRan = new Set<Suite>();
 
   // Indexed sweep, not for..of: anything that registers DURING the run (a
@@ -420,6 +460,7 @@ setTimeout(async () => {
     const fullName = getFullName(t);
     const chain = getSuiteChain(t.suite);
 
+    let testError: any = null;
     try {
       // Run beforeAll for suites that haven't run yet
       for (const suite of chain) {
@@ -429,27 +470,44 @@ setTimeout(async () => {
         }
       }
 
-      // Run beforeEach hooks (outer to inner)
+      // Run beforeEach hooks (outer to inner). A beforeEach throw skips the
+      // test body (matching node/bun) but NOT the afterEach cleanup below.
       for (const suite of chain) {
         for (const hook of suite.beforeEach) await hook();
       }
 
       await t.fn();
+    } catch (e: any) {
+      testError = e;
+    }
 
-      // Run afterEach hooks (inner to outer)
-      for (const suite of [...chain].reverse()) {
-        for (const hook of suite.afterEach) await hook();
+    // afterEach ALWAYS runs, failing test included - node:test and bun:test
+    // both guarantee this (issue #25). Skipping cleanup on failure compounds:
+    // one real failure left a console.error stub installed, every later test
+    // that re-stubbed died with Sinon's "already wrapped", and crank's webkit
+    // run reported 23 failures for 1 bug while burying the real assertion.
+    // Each hook is individually guarded so one throwing cleanup can't skip
+    // the rest; the test's own error stays the reported failure if both threw.
+    for (const suite of [...chain].reverse()) {
+      for (const hook of suite.afterEach) {
+        try {
+          await hook();
+        } catch (e: any) {
+          testError = testError ?? e;
+        }
       }
+    }
 
+    if (testError == null) {
       console.log("✓", fullName);
       globalThis.__LIBUILD_TEST__.passed++;
-    } catch (e: any) {
+    } else {
       console.error("✗", fullName);
-      console.error("  ", e.message);
+      console.error("  ", testError.message);
       globalThis.__LIBUILD_TEST__.failed++;
       globalThis.__LIBUILD_TEST__.errors.push({
         name: fullName,
-        error: e.message || String(e),
+        error: testError.message || String(testError),
       });
     }
   }

src/_test-runner.ts

@@ -989,25 +989,74 @@ ${bundleContent}
 
     await page.goto(`http://localhost:${port}/`);
 
-    // Wait for tests to complete. If the bundle threw before the runner could
-    // even start (ended never fires), this times out - which is a REPORTED
-    // platform failure carrying the captured page errors, not an uncaught
-    // Playwright exception crashing the CLI with the browser left running.
-    try {
-      await page.waitForFunction(
-        () => (globalThis as any).__LIBUILD_TEST__?.ended === true,
-        { timeout }
-      );
-    } catch {
+    // Wait for the run to END, with a STALL timeout rather than a total
+    // budget. `timeout` means "per file" on node/bun, but the browser runs
+    // the whole suite as one bundle - a single absolute deadline meant a
+    // 30-file suite had to finish everything (browser launch included) in
+    // one file's allowance, so big suites "timed out" at the default while
+    // making steady progress (crank's 602 tests on firefox). Progress -
+    // the ready flag, the runner starting, each completed test - resets the
+    // clock; `timeout` ms with NO progress is a genuine hang.
+    let stalled: string | null = null;
+    {
+      let lastProgress = "";
+      let stallAt = Date.now() + timeout;
+      while (true) {
+        const state = await page
+          .evaluate(() => {
+            const t = (globalThis as any).__LIBUILD_TEST__;
+            return {
+              loaded: t != null,
+              ready: (globalThis as any).__LIBUILD_TEST_READY__ === true,
+              started: t?.started === true,
+              ended: t?.ended === true,
+              done: t ? t.passed + t.failed + (t.skipped ?? 0) : 0,
+              loadErrors: t ? t.loadErrors.length : 0,
+            };
+          })
+          .catch(() => null); // navigation/crash mid-poll: treated as no progress
+        if (state?.ended) break;
+        // A load error before the ready flag is unrecoverable - a module body
+        // threw, so the entry (and the runner) can never start. Fail NOW with
+        // the error, not after a full stall timeout of silence.
+        if (state && !state.ready && state.loadErrors > 0) {
+          stalled = "a test file threw while loading - the runner can never start";
+          break;
+        }
+        const progress = JSON.stringify(state);
+        if (state && progress !== lastProgress) {
+          lastProgress = progress;
+          stallAt = Date.now() + timeout;
+        }
+        if (Date.now() > stallAt) {
+          stalled = !state?.loaded
+            ? "the bundle never evaluated (crashed at load?)"
+            : !state.started
+              ? "the runner never started - a test file's top-level await may be hung"
+              : `stalled after ${state.done} test(s) completed`;
+          break;
+        }
+        await new Promise((r) => setTimeout(r, 200));
+      }
+    }
+
+    if (stalled) {
+      // Best effort: the page-side capture usually has the precise error even
+      // when Playwright's pageerror missed or mangled it.
+      const inPage: string[] = await page
+        .evaluate(() => (globalThis as any).__LIBUILD_TEST__?.loadErrors ?? [])
+        .catch(() => []);
+      const detail = [...new Set([...inPage, ...pageErrors])];
       return {
         platform: platformName,
         passed: 0,
         failed: 1,
         errors: [{
           name: "test run never completed",
-          error: pageErrors.length
-            ? `page error(s) during load/run:\n${pageErrors.map((e) => `  ${e}`).join("\n")}`
-            : `no result within ${timeout}ms - the bundle may have hung or failed to start`,
+          error: `${stalled}` +
+            (detail.length
+              ? `\nerror(s):\n${detail.map((e) => `  ${e}`).join("\n")}`
+              : ` (no progress for ${timeout}ms)`),
         }],
       };
     }
@@ -1023,16 +1072,24 @@ ${bundleContent}
     const errors: Array<{ name: string; error: string }> = [...results.errors];
     let failed = results.failed;
 
-    // An uncaught page error is a failure even when tests passed around it:
-    // in a single browser bundle, one file throwing mid-load aborts every
-    // LATER file's registrations while the earlier tests still run green.
-    for (const e of pageErrors) {
+    // Phase-aware uncaught errors, recorded IN the page (see noteUncaught in
+    // _test-browser.ts). Before the runner started: a test file's body
+    // aborted mid-registration, everything after it silently never
+    // registered - fail. During the run: registration was already complete
+    // (the runner starts on the entry's ready flag), so nothing was lost -
+    // surface as warnings. Suites that deliberately float unhandled
+    // rejections (error-propagation tests) stay green, and behavior no
+    // longer depends on which browser routes rejections to `pageerror`.
+    for (const e of results.loadErrors ?? []) {
       failed++;
       errors.push({
-        name: "uncaught page error",
+        name: "uncaught error during load",
         error: `${e} - tests registered after this error never ran`,
       });
     }
+    for (const e of results.runtimeErrors ?? []) {
+      console.warn(yellow(`⚠ uncaught during run (not a failure): ${e}`));
+    }
 
     // Zero tests out of discovered test files fails: the registration-loss
     // class of bug (see the ready-flag comment in _test-browser.ts) produced

test/test-runner.test.ts

@@ -587,6 +587,14 @@ test("browser bundle parses and contains no live node/bun imports", async () =>
   // dispatcher's TLA continuations run test-file bodies -> zero tests register.
   expect(content).toMatch(/setTimeout\(async/);
   expect(content).not.toMatch(/queueMicrotask\(async/);
+  // Uncaught errors are captured IN the page with the phase known
+  // synchronously: pre-start errors mean lost registrations (fatal), run-time
+  // unhandled rejections are warnings - and behavior no longer depends on
+  // which browser routes rejections to Playwright's pageerror.
+  expect(content).toMatch(/addEventListener\("error"/);
+  expect(content).toMatch(/addEventListener\("unhandledrejection"/);
+  expect(content).toMatch(/loadErrors/);
+  expect(content).toMatch(/runtimeErrors/);
 
   await removeTempDir(testDir);
 });