diff --git a/CHANGELOG.md b/CHANGELOG.md index f1341a8..4736a2f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/package.json b/package.json index b24bb51..7db86f4 100644 --- a/package.json +++ b/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": { diff --git a/src/_test-browser.ts b/src/_test-browser.ts index a4cbd73..dfda3be 100644 --- a/src/_test-browser.ts +++ b/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; type HookFn = () => void | Promise; @@ -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(); // 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), }); } } diff --git a/src/_test-runner.ts b/src/_test-runner.ts index 2a3e27f..95fa24a 100644 --- a/src/_test-runner.ts +++ b/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 diff --git a/test/test-runner.test.ts b/test/test-runner.test.ts index f794f10..a1a22b1 100644 --- a/test/test-runner.test.ts +++ b/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); });