Test runner hardening: browser bundling fixes, esbuild service recovery, x-of-y timeout reporting (0.2.17) - #20
Merged
Merged
Conversation
esbuild keeps one long-lived service child per process and never respawns it. If that child dies (OOM-killed, reaped under load), every subsequent build() in the process throws "The service is no longer running" - one transient death turned a green run of libuild's own suite into 139 failures, all long after the event. Route all builds through a wrapper that, on that specific error only, calls esbuild.stop() to drop the dead handle and retries once (the next build() spawns a fresh service). Verified against a real service child SIGKILLed mid-run: a plain retry stays dead; a retry after stop() recovers. Ordinary build errors surface on the first try. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four fixes shaken out by crank PR #375 and a zendb report against 0.2.16: - Stub bun:test/node:test/expect/fs/pretty-format inside browser bundles instead of externalizing them. Live external dynamic imports forced the dispatcher into a lazy async wrapper; on some esbuild versions the resulting `await init_test()` landed in a non-async wrapper and the whole bundle was a syntax error. Stubs (throwing module bodies) link cleanly and keep the dispatcher's TLA at genuine top level. - Start the browser runner in a macrotask. The dispatcher's TLA defers every test file's body to a microtask continuation; queueMicrotask was already queued ahead of them, so the runner fired with zero tests registered and reported a green empty run. - A browser run that registers zero tests out of discovered files is now a hard failure (runner-malfunction signature), never exit 0. - Emit node/bun bundles as .mjs so packages without "type": "module" can load them. Plus: timeout failures now report "x of y test(s) finished" - the test proxy counts registrations and emits the total on stdout (debounced to a microtask so one line carries the file's final count), the runner parses it back out and strips the marker from displayed output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two Opus reviewers ran adversarially against this branch; every confirmed finding is fixed here: - CRITICAL: esbuild recovery was not single-flight. stop() is process- global and esbuild strands in-flight requests on the service it destroys, so concurrent failing builds each running stop()+retry would kill each other's fresh services and hang the run forever - a strict regression over the flake it fixed. A service-generation counter now ensures only the first failure from a generation recovers. - MAJOR: builds in flight when the service died reject with "The service was stopped" - a different string than the pre-flight guard's - and were never retried. Both spellings now match. - MAJOR: scheduler-based browser-runner starts (queueMicrotask, then setTimeout) all lose some race with top-level await; a test file whose TLA crossed a macrotask had its later tests silently dropped from a green run. The generated entry now sets a ready flag as its last statement - ESM guarantees that runs after every imported file fully evaluates - and the runner waits on it, then sweeps for stragglers. - MAJOR: uncaught page errors now fail the browser run (one file throwing during load aborts every later file's registrations while earlier tests pass), and a bundle that breaks before the runner starts is a reported platform failure instead of an uncaught Playwright timeout that crashed the CLI and leaked the browser. - MAJOR: the registration marker is parsed/stripped as a full line only; a test's own output mentioning it could hijack the denominator and be deleted from the failure dump. - MINOR: .only/.skip/.todo/.failing/.each registrations are now counted (each: one per row) and bun's skip/todo summary lines parsed into the numerator, fixing denominators like "5 of 2"; inconsistent denominators are suppressed. A shard finishing cleanly at the timeout buzzer is no longer misreported as timed out. The browser runner gained test.skip/test.todo/describe.skip so shared files using them don't die mid-load with a TypeError. Adds a real-service integration test: SIGKILL our own esbuild service child (pgrep -P, never machine-wide), assert a plain build stays dead and the wrapped build recovers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two bugs previously noted as out of scope, now in the catchall:
- `libuild test test/` (or running from inside a test directory) found
nothing for files without the .test infix: the `**/test/**` default
glob evaluated from inside that directory demands a nested test/test/.
When the discovery root itself is named test/tests/__tests__,
everything under it now counts - the same set the parent-rooted glob
always claimed to match.
- Browser bundles stubbed `expect`/`pretty-format` for the entire graph,
so a consumer's own browser-test import of them got a libuild-branded
throw instead of the real package. Scoped stubbing now: only imports
FROM libuild's resolved package directory are stubbed (realpath-based,
so linked installs agree with esbuild's importer paths; resolved via
the conditionless ./package.json export because ./test is import-only
and invisible to a require-based resolve). Node builtins and
bun:test/node:test stay stubbed globally - they can never resolve in a
browser bundle, and the deferred throw keeps runtime-guarded
`await import("fs")` consumer patterns building.
Verified e2e in chromium: a consumer test importing pretty-format runs
green with the real package bundled; crank's test/dom.tsx shape is
discovered via `libuild test test`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from two consumer reports: crank PR bikeshaving/crank#375 (both blockers) and a zendb report against 0.2.16.
Browser platform (was: completely broken for some installs)
external, surviving as live dynamic imports that forced a lazy async wrapper; on some esbuild versions the resultingawait init_test()landed in a non-async wrapper. Now stubbed inside the bundle (throwing module bodies), so the dispatcher stays plain ESM with TLA at genuine top level. Regression test syntax-checks the real bundled graph.queueMicrotaskwas queued ahead of them → green empty run. Now a macrotask (setTimeout), which fires after the microtask queue drains.Runner reliability
stop()+ retry on that error only). Verified by SIGKILLing the real service mid-run..mjsso consumers without"type": "module"can load them.Reporting
40 of 47 test(s) finished. The test proxy counts registrations and emits the total on stdout; the runner parses and strips it. Numerator-only fallback for files not importing@b9g/libuild/test.Verification
--platform nodeverified in a package without"type": "module"Not addressedNow addressed in this PR:libuild test test/discovering nothing when files lack the.test.infix, and browser stubs over-applying to consumers' ownexpect/pretty-formatimports.🤖 Generated with Claude Code
https://claude.ai/code/session_01QAc3iDuBb3zN41HrepErUw