Skip to content

Replace playwright-test/uvu with the libuild test runner - #375

Open
brainkim wants to merge 3 commits into
mainfrom
chore/libuild-test
Open

Replace playwright-test/uvu with the libuild test runner#375
brainkim wants to merge 3 commits into
mainfrom
chore/libuild-test

Conversation

@brainkim

@brainkim brainkim commented Aug 12, 2026

Copy link
Copy Markdown
Member

Moves the core suite from playwright-test --runner uvu to libuild test, so crank consumes libuild's test stack alongside its build stack.

Verified on a clean install of libuild 0.2.17, no patching: 602 passed, 3 skipped, exit 0.

What changed

before after
uvu suite() describe()
test.before.each / test.after.each beforeEach / afterEach
uvu/assert (Assert.is, …) expect(...)
playwright-test --runner uvu libuild test test/ --platform <browser>
playwright-test (bundled browsers) playwright (explicit playwright install)

playwright-test.config.js is deleted; CI's browser matrix (chromium/firefox/webkit) is unchanged and now drives --platform.

Each file is wrapped in describe() rather than left flat. That is not cosmetic: uvu's suite() scoped hooks per file, but libuild bundles all 30 files into one root suite, so top-level beforeEach hooks would run before every test in the suite. Left flat, all 602 tests failed with Sinon's "Attempted to wrap error which is already wrapped" — 26 files' hooks each stubbing console.error on top of one another.

Three bugs the conversion shook out

test/jsx-tag.ts has never run. playwright-test.config.js declared input: ["test/*.tsx"], which does not match the one .ts test file. Its 25 tests were silently excluded from every CI run. Including them takes the suite from 577 to 602. They pass.

One assertion was never actually checked. uvu's throws(fn, expects, msg) treats a string second argument as the failure message, not a matcher — only a RegExp or function is compared. So:

Assert.throws(() => renderer.render(<Component />, document.body), "Context iterated twice");

asserted only that something threw. expect().toThrow(string) does substring-match, which surfaced that the real message is <Component> context iterated twice without a yield — lowercase, and differently worded. The expected string is corrected.

Two spots leaned on assertion signatures. uvu types ok(val): asserts val, so Assert.ok(x instanceof Element) narrowed x for later lines. expect().toBeTruthy() does not narrow, so mathml.tsx and svg.tsx now access those values explicitly.

Fidelity

  • 602 test() declarations and 3 test.skips before and after — nothing added or lost. The run reports exactly 602 passed, 3 skipped.
  • Assertion messages, which expect() has no parameter for, are kept as block comments instead of being dropped (expect(x).toBe(1) /* cleanup should be called */;).

libuild bugs this found, fixed in 0.2.17

This PR was blocked on two of them; both are now fixed upstream and the workarounds are gone.

  1. The browser runner started before any test registered. @b9g/libuild/test picks its backend with top-level await, making it an async module, so every importing test file's body is deferred past the runner's queueMicrotask start. It ran with zero tests and reported a green empty run. Fixed by having the generated entry set a ready flag as its last statement, which ESM guarantees runs after every imported file has evaluated.

  2. A zero-test run exited 0. Merging this against 0.2.16 would have silently disabled crank's entire suite with CI green. Now a hard failure — verified here: an empty selection exits 1 with "test files were bundled and loaded, but zero tests registered".

Also fixed in 0.2.17 and used here: test.skip exists in the browser runner (the three disabled hydration tests use it again instead of a local no-op, and are reported as skipped), and libuild test test/ discovers files correctly when the root is itself named test/ (so discovery is the plain directory form rather than explicit globs).

Verification

602 passed, 3 skipped, 0 failed on chromium against a clean install with no patching, exit 0. tsc --noEmit clean, eslint clean. A deliberately empty selection exits 1.

🤖 Generated with Claude Code

https://claude.ai/code/session_019pggktip8wsuxzy2VCY9p7

Moves the core suite from `playwright-test --runner uvu` to
`libuild test --platform <browser>`, so crank consumes libuild's test
stack alongside its build stack.

The conversion is mechanical: uvu's suite() becomes describe(), its
before/after.each become beforeEach/afterEach, and uvu/assert becomes
expect. Each file is wrapped in describe() rather than left flat --
uvu's suite() scoped hooks per file, but libuild bundles every file into
one root suite, so unwrapped hooks would run before every test in the
suite.

Three fixes the conversion forced out:

- test/jsx-tag.ts never ran. playwright-test.config.js selected
  `test/*.tsx`, which excluded the one .ts test file. Those 25 tests now
  run, taking the suite from 577 to 602.

- "multiple iterations without a yield throw" asserted the wrong
  message. uvu's throws() treats a string second argument as the failure
  message, not a matcher, so "Context iterated twice" was never compared
  against the actual "<Component> context iterated twice without a
  yield". expect().toThrow() does compare it, and the string is fixed.

- Two spots relied on uvu's assertion signatures (`ok(x): asserts x`) to
  narrow types. expect() does not narrow, so those reads are now
  explicit.

Assertion messages that expect() has no argument for are preserved as
block comments rather than dropped. The three previously test.skip'd
hydration tests are held in a local no-op, since libuild's browser
runner has no skip.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@brainkim

Copy link
Copy Markdown
Member Author

@b9g/libuild@0.2.17 is published and should unblock this draft — both blockers are fixed, along with the minor note:

Blocker 1 (runner fires before registration): fixed, but not with the setTimeout patch — any scheduler-based start still loses to a test file whose top-level await crosses a macrotask (its later tests silently vanish from a green run). Instead the generated entry now sets a ready flag as its last statement — ESM guarantees that runs only after every imported file, TLA continuations included, has fully evaluated — and the runner waits on it. Your 602-passing conversion was produced under the equivalent patch, so it should pass unchanged; the local libuild patch can be dropped.

Blocker 2 (zero tests exits 0): a browser run that discovers files but registers zero tests now exits 1.

Minor (libuild test test/ finds nothing): fixed — when the discovery root itself is named test/tests/__tests__, files without the .test. infix (e.g. test/dom.tsx) are discovered. The explicit-glob invocation in this PR also still works.

Two things that may affect the diff here:

  • The browser runner now has test.skip/test.todo/describe.skip (they register nothing and tally as skipped), so the local no-op skip() shim for the three hydration tests can go.
  • Browser runs are stricter than what this PR was tested against: uncaught page errors now fail the run (previously they were log lines while the run stayed green), and a bundle that breaks before the runner starts is a reported failure. If anything in the suite was quietly relying on a swallowed page error, it will surface as red — that's the fix working, not a regression.

Full details in the release notes: https://github.com/bikeshaving/libuild/releases/tag/v0.2.17

0.2.17 fixes both bugs this migration surfaced: the browser runner no
longer starts before test files register, and a run that registers zero
tests is now a hard failure instead of a green exit 0.

Two workarounds go away with it. The three disabled hydration tests use
test.skip() again rather than a local no-op, since the browser runner now
has it -- and they are reported as skipped, so the suite totals 602
passed and 3 skipped, matching the 602 test() and 3 test.skip
declarations in the source exactly. Discovery is back to `libuild test
test/`, the directory form having been fixed for roots named test/.

Verified against a clean install with no patching: 602 passed, 3 skipped,
exit 0; a deliberately empty selection exits 1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@brainkim
brainkim marked this pull request as ready for review August 12, 2026 08:15
Picks up the node-backend fixes from bikeshaving/libuild#23. No change
for this suite, which runs on chromium: still 602 passed, 3 skipped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
brainkim added a commit that referenced this pull request Aug 12, 2026
…yield

When an async generator component calls refresh() while its next iteration
is still pending, the yielded children are stale before they commit: the
refresh enqueues a run which immediately resumes the generator. Whether
the stale children ever reached the DOM depended on a race between two
microtask chains -- the parent's commit walk and the enqueued run's diff --
whose relative order is not specified. V8 and older JavaScriptCore
resolved the race one way (stale children never appear); JavaScriptCore
as of Safari 26 resolves .finally() two ticks faster than V8 (measured:
Promise.resolve().finally().then() lands on tick 2 vs tick 4), which
flips it: the stale children flash into the DOM and the render promise
resolves with them. Caught by "async generator refresh during await with
direct yield" in test/cascades.tsx on webkit 26.5, which playwright-test
pins too old to see.

The fix makes the supersede explicit instead of racing for it. refresh()
sets an IsSupersededByRefresh flag when a run is already in flight; when
the pending iteration then arrives, its children are not diffed at all
and the parent's diff resolves with the enqueued run's diff instead. The
block chain is derived from the iteration via a closure rather than from
the parent diff, because the enqueued run only starts once the block
settles -- deriving it from the now-chasing parent diff would deadlock.

The promise graph is unchanged whenever no supersede occurs. This
matters: external re-renders must coalesce without chasing (the
"for...of enqueues" test: the in-flight run commits its own children),
and several tests sample the DOM mid-flight and encode the exact
microtask hop counts of the current implementation.

Verified: 577 passed (chromium, playwright-test), 602 passed 0 failed on
webkit 2336 and webkit 2191 and chromium (libuild runner on the #375
branch with this crank.ts), and an instrumented trace shows identical
mutation order on V8 and new JSC: a single commit of the fresh children,
with the stale children never appearing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant