Skip to content

chore(test): fail fast when Bun predates —parallel test isolation#392

Merged
manovotny merged 3 commits into
mainfrom
manovotny/ecstatic-curie-00446f
Jul 25, 2026
Merged

chore(test): fail fast when Bun predates —parallel test isolation#392
manovotny merged 3 commits into
mainfrom
manovotny/ecstatic-curie-00446f

Conversation

@manovotny

@manovotny manovotny commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

On Bun older than 1.3.13, bun run test reports hundreds of failures on a clean main — 1811 pass / 303 fail on Bun 1.3.11. The --parallel/--isolate flags don't exist before 1.3.13, so Bun silently ignores them, runs all 143 test files in one process, and lets mock.module() registrations bleed across files. CI is green only because setup-bun installs the latest Bun. This raises the floor and adds a preflight so a stale Bun fails loudly with an upgrade hint instead of a confusing test cascade.

Changes

  • Raise engines.bun to >=1.3.13, the first version where bun test --parallel (which implies --isolate) exists. Confirmed against the official 1.3.12 and 1.3.13 binaries — the flags land in 1.3.13.
  • Add scripts/check-bun-version.ts, run first by both test and test:e2e. Bun does not enforce engines.bun at install time (verified on 1.3.11 and 1.3.13), so this is the only mechanism that catches a stale Bun before the suite runs. It reads the floor from engines.bun, so there's one source of truth. Co-located test covers the version matrix.
  • Sync docs that still described scripts/run-tests.ts, removed in chore(test): pin fixture deps and manifest-drive the e2e harness #280: CLAUDE.md, .claude/rules/testing.md, .claude/rules/scripts.md, .claude/rules/e2e.md, CONTRIBUTING.md, and the scripts/run-e2e-op.ts JSDoc. These are loaded as agent instructions, so a stale runner reference sends both agents and contributors after a file that no longer exists.
  • Fix the sharpest doc trap: the guides documented test:e2e:op -- --filter react to scope a run, but a forwarded positional pattern is OR'd with test/e2e/ and broadens the run to the entire suite. E2E is the 30-minute Playwright suite that creates real users against the production Clerk API. Replaced the removed runner's --filter/--debug/--har flags with their env-var equivalents, and pointed the single-fixture example at the test file (test/e2e/react.test.ts) rather than the fixture project dir, which matched nothing.

Not planned

  • No pin added to ci.yml — setup-bun already installs a Bun newer than the floor, and the preflight now catches any environment that isn't. A pin would be one more thing to bump.
  • Left the deliberate Bun 1.3.11 pin in build-binaries.yml alone (it's there for a darwin-arm64 codesign issue). Added one line to docs/releasing.md noting that pin only covers compiling and sits below the test floor.
  • Left the broader fixture-authoring docs alone — e2e.md's "How fixtures work" / "Adding a new fixture" and CONTRIBUTING's fixture description still predate chore(test): pin fixture deps and manifest-drive the e2e harness #280's manifest-driven harness (config in fixtures.manifest.ts, npm ci, createFixtureHarness). That's a separate subsystem doc migration; tracked as a follow-up.

Verification

Full unit suite on Bun 1.3.13: 2123 pass / 0 fail across 144 files. format, lint, and typecheck all clean. On Bun 1.3.11, bun run test now stops immediately with the required range, the found version, and bun upgrade.

bun test --parallel (and --isolate) first shipped in Bun 1.3.13. Older
Bun silently ignores the flags, runs every test file in one process, and
lets mock.module() registrations bleed across files — surfacing as
hundreds of order-dependent failures. Bun does not enforce engines.bun at
install time, so raise the floor to >=1.3.13 and add a preflight
(scripts/check-bun-version.ts) that both test scripts run first, failing
loudly with an upgrade hint instead of a confusing test cascade.

Also sync stale docs that still referenced the removed scripts/run-tests.ts
runner (gone since #280) and its flags — notably e2e.md, which documented
`test:e2e:op -- --filter react` to scope a run when a forwarded positional
pattern now broadens the run to the whole (expensive) e2e suite instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: f08ed58

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

manovotny and others added 2 commits July 23, 2026 15:43
- e2e.md: the single-fixture example targeted the fixture project dir
  (test/e2e/fixtures/react/), which matches no tests; point at the test
  file (test/e2e/react.test.ts) instead, and fix the patch filename
  (1.58.2 -> 1.60.0).
- CLAUDE.md: CI runs `bun run test`, not a bare `bun test`.
- CONTRIBUTING.md: steer contributors to `bun run test` (isolation +
  version preflight) over bare `bun test`, and replace the removed
  runner's --filter/--debug/--har flags with the env-var equivalents.
- run-e2e-op.ts: the JSDoc example used the dead --filter flag; forwarded
  args now go to `bun test`, where a positional path broadens the run.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `-t "sign in"` example matched no test name. The registered names
are "project builds with no errors", "typecheck passes with no errors",
and "app loads and auth flow works" — use "project builds".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@manovotny
manovotny marked this pull request as ready for review July 23, 2026 21:07
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds check-bun-version.ts and tests it against semver ranges, then runs it before the unit and E2E test scripts. Raises the minimum Bun engine version to 1.3.13 and adjusts unit-test targets. Updates contributor, CI, release, and rule documentation for parallel test isolation, E2E argument forwarding, fixture concurrency, Playwright requirements, and revised script references.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: wyattjoh

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: failing fast on Bun versions that predate parallel test isolation.
Description check ✅ Passed The description is detailed and directly matches the changeset, covering the Bun floor, preflight check, and docs updates.

Comment @coderabbitai help to get the list of available commands.

@manovotny manovotny changed the title chore(test): fail fast when Bun predates --parallel test isolation chore(test): fail fast when Bun predates —parallel test isolation Jul 23, 2026
rafa-thayto pushed a commit that referenced this pull request Jul 24, 2026
#393)

The E2E fixture-authoring docs still described the pre-#280 harness. Update
them to match the current manifest-driven setup:

- Fixture config now lives in test/e2e/fixtures.manifest.ts (keyed by name),
  not exported from each test file; tests call createFixtureHarness(name).
- Correct the exported helpers to the plural runFixtureTests()/runBrowserTests()
  in CONTRIBUTING.md (were the singular runFixtureTest()/runBrowserTest()).
- Fixture setup uses `npm ci` (fixtures migrated from bun to npm with a
  checked-in package-lock.json), not `bun install`.
- Refresh the FixtureConfig field list (drop removed `description`/`pinned`,
  add `pinnedDependencyRanges`/`packageJsonOverrides`) and drop the stale
  `--force`/non-pinned refresh flags (the script only accepts `--only`).

Follow-up to #392, which deliberately scoped this rewrite out.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@manovotny
manovotny merged commit e2224f0 into main Jul 25, 2026
10 checks passed
@manovotny
manovotny deleted the manovotny/ecstatic-curie-00446f branch July 25, 2026 00:26
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.

2 participants