test: adopt bun test --isolate --parallel for ~2.9x CI speedup#839
Merged
Conversation
Bun 1.3.13 shipped `--isolate` (fresh global per file) and `--parallel` (worker process per CPU). Adopting both reduces local wall clock from ~152s to ~52s on a 4-core box and collapses the test:unit + test:isolated split that existed only to work around mock.module() registry pollution (bun:test#258) — every file now gets a clean module graph. ## Preload race fix Worker processes inherit a shared `TEST_TMP_DIR = /tmp/sentry-cli-test` from `test/constants.ts`. Each worker runs the preload on startup, which includes `rmSync(TEST_TMP_DIR, { recursive, force })` to clear stale state from crashes. Under `--parallel`, workers would wipe each other's DB files mid-run, surfacing as `SQLITE_IOERR_DELETE_NOENT` flakes. Fix: namespace `TEST_TMP_DIR` by `BUN_TEST_WORKER_ID` — each worker gets `/tmp/sentry-cli-test-w${id}` and the wipe stays worker-local. Serial runs (no worker ID) behave exactly as before. Tests with fixed-name subdirs (`upgrade-lock-test`, etc.) are automatically unique per worker because the parent path is worker-scoped. ## Stability Stress-tested with 5 consecutive full runs of `--isolate --parallel` on the merged suite (test/lib + test/commands + test/types + test/isolated): 5920/5920 pass each time, ~52-55s wall clock. The subprocess-heavy `test/lib/db/concurrent.test.ts` also passes cleanly under parallel — SQLite's WAL + busy_timeout pragmas handle the contention. ## CI changes Drops the separate `Isolated Tests` step and `Merge Coverage Reports` step — everything now runs through `test:unit` with a single coverage file. Removes the now-unused `script/merge-lcov.ts`. ## Not doing here (deferred) - Collapse `test/isolated/` folder into `test/lib`/`test/commands` - Remove `scopedFetchMock` helper (the defensive marker-based filter in `test/lib/sentry-client.test.ts`) — still correct under --isolate, not worth churning in this PR - Add `test:shard` for CI matrix splitting — current wall clock is fine ## Verification - `bun run test:unit` → 5920 pass, 0 fail, coverage/lcov.info emitted - Stress loop: 5 consecutive clean runs - `bun run typecheck` passes - `bun run lint` passes (1 pre-existing warning, unrelated)
Contributor
Codecov Results 📊✅ 5920 passed | Total: 5920 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
All tests are passing successfully. ❌ Patch coverage is 75.00%. Project has 13391 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 95.32% 74.64% -20.68%
==========================================
Files 284 284 —
Lines 41349 52805 +11456
Branches 0 0 —
==========================================
+ Hits 39415 39414 -1
- Misses 1934 13391 +11457
- Partials 0 0 —Generated by Codecov Action |
BYK
added a commit
that referenced
this pull request
Apr 24, 2026
Follow-up to #839. With `bun test --isolate` giving each file a fresh module graph, the `test/isolated/` split that existed to work around `mock.module()` registry pollution (oven-sh/cli#258) is no longer needed. ## Merged into existing files Per-feature tests now live with their natural counterparts: | From | To | |------|-----| | `isolated/brew-upgrade.test.ts` | `test/lib/upgrade.test.ts` | | `isolated/log-view-prompt.test.ts` | `test/commands/log/view.test.ts` | | `isolated/login-reauth.test.ts` | `test/commands/auth/login.test.ts` | | `isolated/project-delete-confirm.test.ts` | `test/commands/project/delete.test.ts` | | `isolated/set-commits-auto.test.ts` | `test/lib/api/releases.test.ts` | | `isolated/set-commits-auto-no-remote.test.ts` | `test/lib/api/releases.test.ts` (merged together — `getRepositoryName` is now a controllable `mock()`) | | `isolated/dsn/project-root.test.ts` | `test/lib/dsn/project-root.test.ts` | Pattern: `mock.module()` at the top of the merged file, followed by a dynamic `await import()` for the code under test so the mocked bindings win. Other static imports stay as-is. Also unblocks the previously-deferred stub in `test/lib/api/releases.test.ts` that said _"setCommitsAuto tests live in test/isolated/ because they require mock.module() for git helpers"_. ## Moved as sibling files Two isolated files tested the same subjects as existing files but with incompatible mocking strategies. Under `--isolate` they coexist safely as siblings: | From | To | |------|-----| | `isolated/resolve-target.test.ts` | `test/lib/resolve-target.mocked.test.ts` | | `isolated/delta-upgrade.test.ts` | `test/lib/delta-upgrade.mocked.test.ts` | The existing `resolve-target.test.ts` and `delta-upgrade.test.ts` use real DB + mocked fetch; the `.mocked.` siblings stub every collaborator via `mock.module()`. ## New files (no prior counterpart) Three isolated DSN helpers had no target file in `test/lib/dsn/`: - `isolated/dsn/errors.test.ts` → `test/lib/dsn/errors.test.ts` - `isolated/dsn/fs-utils.test.ts` → `test/lib/dsn/fs-utils.test.ts` - `isolated/dsn/resolver.test.ts` → `test/lib/dsn/resolver.test.ts` ## Removed `scopedFetchMock` helper `test/lib/sentry-client.test.ts` previously wrapped all its fetch mocks with `scopedFetchMock(marker, handler)` to filter out foreign fetch calls that leaked from other test files (CI run 24835339085). Under `bun test --isolate` each file gets a fresh `globalThis.fetch`, so cross-file leakage is eliminated. The 6 call sites now use `mockFetch()` directly; the helper and its `urlOf()` dependency are gone (~30 lines). ## Other cleanups - Removed `test/isolated/` directory - Updated `package.json` `test:unit` to drop `test/isolated` from the argument list - `test/lib/dsn/project-root.test.ts`: `noopSpan` stub now includes `setAttributes` (production code calls both `setAttribute` and `setAttributes`) - Replaced `() => {}` test stubs with named `noop` functions to satisfy `lint/suspicious/noEmptyBlockStatements` without per-line biome-ignore comments ## Verification - ✅ `bun run test:unit` — 5920 pass / 0 fail across 253 files (was 260 — merges consolidated 12 files into 5 new + 7 existing) - ✅ 3 consecutive stress runs clean - ✅ `bun run typecheck`, `bun run lint` Net diff: **+1219 / −3830** (most "deletions" are file moves; real code reduction is the `scopedFetchMock` removal plus deduplication from merging).
5 tasks
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.
Summary
Bun 1.3.13 shipped
--isolate(fresh global per file) and--parallel(worker process per CPU). Adopting both cuts local wall clock from ~152s to ~52s on a 4-core box and collapses thetest:unit+test:isolatedsplit that existed only as a workaround formock.module()registry pollution (oven-sh/bun#258) — every file now gets a clean module graph.Preload race fix
Worker processes inherit a shared
TEST_TMP_DIR = /tmp/sentry-cli-testfromtest/constants.ts. Each worker runs the preload on startup, which includesrmSync(TEST_TMP_DIR, { recursive, force })to clear stale state from crashes. Under--parallel, workers would wipe each other's DB files mid-run, surfacing asSQLITE_IOERR_DELETE_NOENTflakes.Fix: namespace
TEST_TMP_DIRbyBUN_TEST_WORKER_ID— each worker gets/tmp/sentry-cli-test-w${id}and the wipe stays worker-local. Serial runs (no worker ID) behave exactly as before. Tests with fixed-name subdirs (upgrade-lock-test, etc.) are automatically unique per worker because the parent is worker-scoped.Stability
Stress-tested with 5 consecutive full runs of the merged suite (
test/lib + test/commands + test/types + test/isolated) under--isolate --parallel: 5920/5920 pass each time, ~52-55s wall clock. The subprocess-heavytest/lib/db/concurrent.test.tsalso passes cleanly — SQLite's WAL + busy_timeout pragmas handle the contention.CI changes
Isolated Testsstep andMerge Coverage Reportsstep — everything runs throughtest:unitnowcoverage/lcov.infofilescript/merge-lcov.tsScript changes
test: alias fortest:unit(wastest:unit && test:isolated)test:unit: now runs the merged directory tree with--isolate --paralleltest:isolated: removed (folder still exists; files run under--isolateviatest:unit)test:changed: new dev-only script usingbun test --isolate --changedDeferred to follow-ups
test/isolated/folder intotest/lib/test/commands(churn-heavy)scopedFetchMockhelper (test/lib/sentry-client.test.ts) — still correct under--isolate, just no longer strictly neededtest:shardfor CI matrix splitting — current wall clock is fineVerification
bun run test:unit— 5920 pass / 0 fail, coverage/lcov.info emittedbun run typecheckbun run lint(1 pre-existing warning, unrelated)Relying on the CI matrix for macOS and Windows coverage of the new flags.