Skip to content

[CI] Speed up unit tests with cross-runtime duration scheduling - #25967

Merged
nwparker merged 4 commits into
mainfrom
nwparker/bun-test-scheduling
Oct 6, 2026
Merged

nwparker merged 4 commits into
mainfrom
nwparker/bun-test-scheduling

Conversation

@nwparker

@nwparker nwparker commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

Start slow tests early across Bun and Node, instead of finishing Bun's project before beginning Node's. Every test still runs in its own isolated worker.

What Changed

  • Reuse CI's recorded file durations to order both runtime projects together. Preserve explicit execution groups, isolation priority, shard membership and Node-only fallback ordering.
  • Upgrade to Vitest 5.0.3 while explicitly retaining the previous mock-clearing and project-inheritance behavior. Adapt benchmark callbacks and clean up asynchronous fixture resources before deleting their temporary directories.
  • Keep real SQLite fixtures and V8 heap-retention contracts on Node, with source checks that catch new suites missing from that partition. Bun continues to run the remaining suites.
  • Emit the packaged-CLI fixture without repeating the separately required CLI typecheck. Identical emitted JavaScript; the focused fixture fell from 1.50s to 0.55s.
  • Fix a YAML configuration bug exposed by the newer assertions: merged defaults now retain their inherited command. The test checks all 40 tabs. The separate merge-expansion bound gap identified in review is fixed by merged [CI] Bound YAML merge conversion and close SQLite routing review gaps #25998, including tagged-map and direct-pair regression coverage.

Why

Measured before/after on macOS arm64 (M4 Max, Node 24.20.0, Bun 1.4.2, 16 file workers):

Measured change Before After Less wall time
Scheduling only, same Vitest 5 / Bun+Node sample 55.016s, project-first 48.326s, cross-runtime duration order 12.16%
Vitest version only, same duration-ordered Bun+Node sample 52.447s, Vitest 4.1.11 48.326s, Vitest 5.0.3 7.86%
Packaged-CLI fixture emission 1.504s, with repeated typecheck 0.547s, emit-only 63.63%
Cumulative full-suite migration, from original Node setup through this PR 542.496s 476.583s 12.15%

Both 1,000-file samples passed with 9,712 cases, identical per-file outcomes and 71 existing skips. The component improvements are separate experiments and must not be added together. The CLI experiment emitted all 3,300 JavaScript outputs byte-identically; its separate required typecheck remains in CI.

The full-suite row is cumulative across #25840 and this PR, not an isolated before/after measurement of this PR alone. It uses the frozen earlier checkout: all 11,048 files and prior cases remained, and one additional routing-policy assertion brought the after total to 108,364. The 849 existing skips were unchanged; both runs passed. The 2× full-suite target is still unmet. Shared-server, native factory-bridge, cache and alternative transport experiments were slower, incomplete or failed, and are excluded from claimed improvements.

The branch is rebased onto main at 51062e6 and supplies the launch-argument dependency missing from a scripted Codex fixture introduced by newer main commits. Before the latest main update and the additional Node lifecycle routing, its full local suite passed: 11,229 files / 110,105 cases in 606.2s, zero errors. Latest-head validation is listed below; these full-run timings describe the earlier matched checkout. The original Node/Vitest setup collected exactly the same files/cases/skips in 823.9s, but failed two existing filesystem deadlines that the candidate passed; that failed run is not a passing-speedup comparison. The frozen-checkout timings above are not a benchmark of these newly added suites.

Linked Issue

Maintainer follow-up to #25840; no separate issue.

Visual Proof

N/A — test-runner infrastructure and a YAML parsing correction; no rendered UI or interaction changes.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated

macOS arm64: full frozen-checkout qualification; matched sample coverage comparisons; rebased pnpm tc, pnpm lint and pnpm run check:code-quality:changed; scheduling/shard/routing checks; focused YAML, SSH cancellation and Claude child-cleanup tests. The rebased full suite, full pnpm lint and typecheck passed. The same-source Node baseline had two deadline failures. The Bun/Vitest 4 full comparison collected identical coverage in 692.0s but failed one existing watcher deadline; it is not a passing-speedup comparison. Latest-main typecheck, full lint, and all 23 additionally routed store-lifecycle suites (139 cases) passed. Exact-head CI completed with 35 successful checks, five existing conditional skips and no failures. All five unit shards and Linux/macOS/Windows platform checks passed. Real-desktop computer tests remain paused locally.

Review

Fixture cleanup, runtime routing, sorting and coverage reviewed. The two post-merge bot findings are fixed by #25998, merged as 3308ff8b269a828738819f85f72841e8170bf895: pre-conversion YAML merge bounds (including !!omap/!!pairs) and SQLite classification for mixed default/type-only bindings and empty imports. All five unit shards and the complete current-head PR workflow passed; CodeRabbit reviewed that fix head with no actionable comments. Both original review threads now link the merged proof and are resolved. The earlier module-mocking comment was fixed by dependency injection before merge. Paths use Node path utilities; the Node worker uses the existing cross-platform process launcher. No application runtime, SSH protocol or remote execution-boundary migration.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer code or fixtures copied.

Notes

Preserve Node's SQLite and native-module behavior. No removed assertions, added skips, relaxed deadlines, reduced workloads, dependency patches, native loader or worker reuse.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Visual proof is N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform and SSH/remote impact considered
  • CI checks pass; 35 successful checks, five existing conditional skips, no failures

@nwparker
nwparker marked this pull request as ready for review October 6, 2026 21:25
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

The Vitest configuration now uses runtime-aware sequencing and includes additional Node runtime and SQLite test files. New boundary tests check runtime include membership. Test fixtures and assertions were updated across main-process and renderer tests. The YAML parser now processes merge keys, with expanded coverage for merged tab values.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 67c91

The SQLite runtime guard can miss some runtime imports, and the YAML alias-limit concern remains open. These warrant owner awareness and follow-up, but the supplied evidence does not establish a broader failure that blocks merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 27 files. 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: speeding up unit tests by scheduling work across runtimes using duration data.
Description check ✅ Passed The description covers the required sections, explains the change and rationale, identifies the related issue, gives a reason for omitting visual proof, and reports testing and review details. It also…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5aa3e4bf-fd47-424a-9714-7aa282574d9e
📥 Commits

Reviewing files that changed from the base of the PR and between 0a26f3e and 6624610dd3041fddca510083d2b9886958303ddc.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (27)
  • config/scripts/ci-shard-assignment.mjs
  • config/scripts/ci-shard-assignment.test.mjs
  • config/scripts/ci-unit-sequencer.mjs
  • config/scripts/vitest-host-ports-setup.ts
  • config/scripts/vitest-node-runtime-boundary.test.ts
  • config/scripts/vitest-node-runtime-files.mjs
  • config/scripts/vitest-runtime-sequencer.mjs
  • config/scripts/vitest-runtime-sequencer.test.mjs
  • config/scripts/vitest-sqlite-database-runtime-files.mjs
  • config/scripts/vitest-sqlite-runtime-boundary.test.ts
  • config/scripts/vitest-sqlite-runtime-files.mjs
  • config/vitest.config.ts
  • package.json
  • src/main/claude/claude-stream-json-connection.test.ts
  • src/main/cli/packaged-cli-assets.test.ts
  • src/main/runtime/orca-runtime-test-mocks.spec.ts
  • src/main/runtime/orca-runtime-test-mocks/imported-values.spec.ts
  • src/main/skills/skill-upload-session-service.test.ts
  • src/main/ssh/ssh-channel-multiplexer.test.ts
  • src/renderer/src/components/dashboard/DashboardAgentChildDisclosure.test.tsx
  • src/renderer/src/components/editor/markdown-preview-review-frame-lifecycle.test.tsx
  • src/renderer/src/components/editor/monaco-content-sync.bench.ts
  • src/renderer/src/components/editor/monaco-content-sync.undo-retention.bench.ts
  • src/renderer/src/components/network/CustomAddressDialog.test.tsx
  • src/renderer/src/components/status-bar/UsagePercentageDisplayChangeNotice.test.tsx
  • src/shared/orca-yaml-alias-bounds.test.ts
  • src/shared/orca-yaml.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread src/shared/orca-yaml.ts Resolved
@nwparker
nwparker force-pushed the nwparker/bun-test-scheduling branch from 85ed872 to 67c9174 Compare October 6, 2026 22:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f2bf9ad9-fec7-40c9-a426-72937db12585
📥 Commits

Reviewing files that changed from the base of the PR and between 6624610dd3041fddca510083d2b9886958303ddc and 67c9174.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • config/scripts/vitest-runtime-sequencer.mjs
  • config/scripts/vitest-runtime-sequencer.test.mjs
  • config/scripts/vitest-sqlite-database-runtime-files.mjs
  • config/scripts/vitest-sqlite-runtime-boundary.test.ts
  • src/main/runtime/structured-agent-session-codex-stopped-send-order.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

@nwparker
nwparker merged commit d8c871a into main Oct 6, 2026
40 checks passed
OrcaWin added a commit that referenced this pull request Oct 7, 2026
…6046)

The runner excluded *.electron.test.ts with a CLI --exclude, but main's switch to Vitest inline
projects (#25967) gave each project its own exclude list, which overrides the CLI one. The
directory selectors then pulled profile-state-writer-stall.electron.test.ts into the glibc-floor
and musl orcad-template jobs, which have no xvfb. Resolve the exact files with vitest list and
drop Electron and cross-runtime ones before running.

Co-authored-by: m4air <m4air@Mac.localdomain>
@nwparker nwparker changed the title Speed up unit tests with cross-runtime duration scheduling [CI] Speed up unit tests with cross-runtime duration scheduling Oct 9, 2026
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