ci: isolate Bun test shards into fresh-process batches - #1469
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughLinux CI replaces direct Bun shard execution with deterministic fresh-process batching. The helper validates configuration, assigns tests to shards, classifies failures, and retries runtime crashes or timeouts per file. Workflow tests verify API and storage test inclusion. ChangesBun CI batching
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant BatchHelper
participant BunTestProcess
CIWorkflow->>BatchHelper: pass TEST_SHARD and batch settings
BatchHelper->>BunTestProcess: run a bounded test batch
BunTestProcess-->>BatchHelper: return status and log output
BatchHelper->>BunTestProcess: retry affected files individually after timeout or runtime crash
BatchHelper-->>CIWorkflow: return shard status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
Wibias
marked this pull request as ready for review
August 11, 2026 10:00
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 236-239: Update the workflow comment describing
scripts/ci/run-bun-test-batches.sh to document both one-time Bun runtime-crash
recovery and the one-time retry for singleton files after a timeout, while
preserving the statement that ordinary test failures are not retried.
In `@tests/zz-ci-api-usage-isolation.test.ts`:
- Around line 28-31: Update the assertions in
tests/zz-ci-api-usage-isolation.test.ts lines 28-31 to verify that the
tests/api-usage.test.ts matching branch returns 1, rather than only checking
filename text. Update tests/zz-ci-storage-policy-isolation.test.ts lines 28-32
to verify that its storage-policy patterns are within the exclusion branch
returning 1; both sites require direct assertion changes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5b9fca9d-3ef0-40fb-91c5-11b128cb21db
📒 Files selected for processing (4)
.github/workflows/ci.ymlscripts/ci/run-bun-test-batches.shtests/zz-ci-api-usage-isolation.test.tstests/zz-ci-storage-policy-isolation.test.ts
Comment on lines
+236
to
+239
| # `scripts/ci/run-bun-test-batches.sh` mirrors Bun's sorted round-robin shard | ||
| # assignment, then runs each shard in small batches so every batch gets a fresh | ||
| # Bun process. The helper prints the exact files before each batch and retries | ||
| # only a Bun runtime crash once; ordinary test failures are never retried. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document timeout retries.
Lines 238-239 state that the helper retries only a Bun runtime crash. scripts/ci/run-bun-test-batches.sh also retries a singleton file once after a timeout. Update this text to describe both runtime-crash and timeout recovery. Otherwise, CI operators can misdiagnose an expected retry as unexpected behavior.
Proposed fix
- # Bun process. The helper prints the exact files before each batch and retries
- # only a Bun runtime crash once; ordinary test failures are never retried.
+ # Bun process. The helper prints the exact files before each batch and isolates
+ # runtime crashes and timeouts per file. Ordinary test failures are never retried.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # `scripts/ci/run-bun-test-batches.sh` mirrors Bun's sorted round-robin shard | |
| # assignment, then runs each shard in small batches so every batch gets a fresh | |
| # Bun process. The helper prints the exact files before each batch and retries | |
| # only a Bun runtime crash once; ordinary test failures are never retried. | |
| # `scripts/ci/run-bun-test-batches.sh` mirrors Bun's sorted round-robin shard | |
| # assignment, then runs each shard in small batches so every batch gets a fresh | |
| # Bun process. The helper prints the exact files before each batch and isolates | |
| # runtime crashes and timeouts per file. Ordinary test failures are never retried. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 236 - 239, Update the workflow comment
describing scripts/ci/run-bun-test-batches.sh to document both one-time Bun
runtime-crash recovery and the one-time retry for singleton files after a
timeout, while preserving the statement that ordinary test failures are not
retried.
Comment on lines
+28
to
+31
| const batchHelper = await Bun.file( | ||
| new URL("../scripts/ci/run-bun-test-batches.sh", import.meta.url), | ||
| ).text(); | ||
| expect(batchHelper).toContain("tests/api-usage.test.ts)"); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the exclusion result, not only filename text.
These assertions pass if the filename appears in a comment or if the matching case branch returns 0. A regression can then run dedicated tests in general shards and remove the intended isolation.
tests/zz-ci-api-usage-isolation.test.ts#L28-L31: assert that thetests/api-usage.test.tsbranch returns1.tests/zz-ci-storage-policy-isolation.test.ts#L28-L32: assert that the storage-policy patterns are in the exclusion branch that returns1.
Proposed fix
- expect(batchHelper).toContain("tests/api-usage.test.ts)");
+ expect(batchHelper).toMatch(
+ /tests\/api-usage\.test\.ts\)\s*\n\s*return 1/,
+ );- expect(batchHelper).toContain("tests/api-storage-policy*.test.ts");
- expect(batchHelper).toContain("tests/api-storage.test.ts");
+ expect(batchHelper).toMatch(
+ /tests\/api-storage-policy\*\.test\.ts\|tests\/api-storage\.test\.ts\|tests\/api-usage\.test\.ts\)\s*\n\s*return 1/,
+ );📍 Affects 2 files
tests/zz-ci-api-usage-isolation.test.ts#L28-L31(this comment)tests/zz-ci-storage-policy-isolation.test.ts#L28-L32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/zz-ci-api-usage-isolation.test.ts` around lines 28 - 31, Update the
assertions in tests/zz-ci-api-usage-isolation.test.ts lines 28-31 to verify that
the tests/api-usage.test.ts matching branch returns 1, rather than only checking
filename text. Update tests/zz-ci-storage-policy-isolation.test.ts lines 28-32
to verify that its storage-policy patterns are within the exclusion branch
returning 1; both sites require direct assertion changes.
3 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 testprocess with deterministic fresh-process batches.storage-policyandapi-usagejobs, plus the unsharded macOS control and dispatch-only Windows lane.BUN_TEST_BATCH_SIZE; the timeout is configurable throughBUN_TEST_BATCH_TIMEOUT_SECONDS.Verification
bash -n scripts/ci/run-bun-test-batches.sh4fb67f8supplied the root-cause evidence for the adaptive fallback:tests/cli-help.test.tsand then hung until the outer 120-second timeout on both attemptstests/cli-status-json.test.tsand then hung until the same timeout on both attemptsbun testinvocation still runs its selected files inside one Bun processfb38f4breplaces the repeated whole-batch retry with singleton process isolation after a runtime crash or timeout, preserving fast 12-file batches on the healthy path.Checklist
Summary by CodeRabbit