-
Notifications
You must be signed in to change notification settings - Fork 95
Migrate remaining workspaces and finalize CI to Bun-native (Fixes #2847) #3014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 30 commits
a71d593
6f05318
e770d90
a385ecb
3ad3154
1f0d2b4
65209b2
7b5e314
52c6282
f7ba44a
e32bf4f
c951375
e0456f4
1f1c863
ff7ad21
dd9552e
3dffdd9
1e4da80
6e38e1a
e1d986e
ade806e
c953c19
78c2115
a985161
45b34b2
fb310c7
a969e7b
581960b
e838e5b
b1e1687
5217763
32845c6
8030ca4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -811,27 +811,34 @@ jobs: | |
| - name: 'Run test orchestrator on a single workspace' | ||
| # Runs the orchestrator against one workspace. --skip-pretest skips | ||
| # pretest hooks (like the agents API-surface guard) to keep the smoke | ||
| # fast. The workspace's test phase still runs (vitest run), but | ||
| # test-utils has a tiny test suite. --skip-scripts skips the root | ||
| # script harness tests. The only failure modes that matter here are | ||
| # bugs in the orchestrator itself (discovery, parsing, routing). | ||
| # fast. The workspace's test phase still runs, but test-utils has a | ||
| # tiny test suite. --skip-scripts skips the root script harness tests. | ||
| # The only failure modes that matter here are bugs in the orchestrator | ||
| # itself (discovery, parsing, routing). | ||
| run: |- | ||
| bun scripts/test.ts --workspace test-utils --skip-scripts | ||
|
|
||
| - name: 'Run script harness tests for the orchestrator' | ||
| run: |- | ||
| bunx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/test-orchestrator.test.ts | ||
| bun test --preload ./test-setup/augment-bun-vi.ts --preload ./scripts/tests/test-setup.ts scripts/tests/test-orchestrator.test.ts | ||
|
|
||
| # | ||
| # Bun-native test compatibility (issue #2475) | ||
| # Bun-native manifest discovery gate (issues #2475, #2847) | ||
| # | ||
| # Runs representative tests from workspaces with supported Bun-native | ||
| # compatibility. Vitest-only workspaces remain covered by the primary test | ||
| # jobs and are intentionally excluded from this smoke gate. | ||
| # Resolves every root declared in scripts/bun-test-manifest.ts without | ||
| # executing anything: globs are expanded, and every selected file, preload, | ||
| # tsconfig override and global-setup module must exist. That is what proves | ||
| # no test file was dropped. | ||
| # | ||
| # It deliberately does NOT run the suite. `test_shard` already executes every | ||
| # root exactly once by invoking each workspace's own `test` script, so a | ||
| # second full run would double the CI bill for no extra signal. The one-owner | ||
| # invariant is enforced by scripts/tests/bun-manifest-root-ownership.bun.test.ts, | ||
| # which fails if a root gains a second executor or loses its only one. | ||
| bun_native_test_parity: | ||
| name: 'Bun Native Test Compatibility' | ||
| runs-on: 'ubuntu-latest' | ||
| timeout-minutes: 30 | ||
| timeout-minutes: 10 | ||
| needs: | ||
| - 'skip_check' | ||
| if: ${{ needs.skip_check.outputs.should_skip != 'true' }} | ||
|
|
@@ -860,9 +867,9 @@ jobs: | |
| bun install | ||
| git checkout -- bun.lock | ||
|
|
||
| - name: 'Run Bun-native tests (manifest-based)' | ||
| - name: 'Resolve every Bun-native root (no execution)' | ||
| run: |- | ||
| bun scripts/run_bun_tests.ts --timeout 30000 | ||
| bun scripts/run_bun_tests.ts --dry-run | ||
|
Comment on lines
+870
to
+872
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 -- 'credentialed|dry-run|dryRun|--root' \
scripts/run_bun_tests.ts \
scripts/bun-test-manifest.tsRepository: vybestack/llxprt-code Length of output: 11426 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- runner resolution and entry point ---'
sed -n '421,470p' scripts/bun-test-manifest.ts
sed -n '600,675p' scripts/run_bun_tests.ts
printf '%s\n' '--- runner call sites ---'
rg -n -C 6 -- 'resolveBunNativeTestFiles|selectsEntry|run_bun_tests' scripts .github/workflows/ci.yml
printf '%s\n' '--- manifest tests and workflow root invocations ---'
rg -n -C 8 -- 'credentialed|selectsEntry|resolveBunNativeTestFiles|--dry-run|--root|--workspace' \
scripts test tests .github/workflows 2>/dev/null || trueRepository: vybestack/llxprt-code Length of output: 50378 Add explicit dry-runs for credentialed roots. Because unfiltered 🤖 Prompt for AI Agents |
||
|
|
||
| # | ||
| # Test: Node (Linux only — issue #2876) | ||
|
|
@@ -1023,8 +1030,9 @@ jobs: | |
| # so local runs keep the signal; the env var is only set in CI. | ||
| LLXPRT_COVERAGE: ${{ (matrix.shard == 'cli' || matrix.shard == 'core') && matrix.os == 'ubuntu-latest' && 'true' || 'false' }} | ||
| # The orchestrator expands --shard to the shard's workspaces (or runs | ||
| # npm run test:scripts for the scripts shard). The tools, mcp, and storage | ||
| # workspaces run native Bun; all other packages use their configured runner. | ||
| # npm run test:scripts for the scripts shard). Each workspace's tests | ||
| # run under whichever runner its own `test` script selects — Bun-native | ||
| # for the migrated workspaces, Vitest for the rest (issue #2578). | ||
| run: 'bun scripts/test.ts --shard "${{ matrix.shard }}"' | ||
|
|
||
| - name: 'Report harness toolchain versions' | ||
|
|
@@ -1057,10 +1065,9 @@ jobs: | |
| run: 'sleep 2' | ||
|
|
||
| - name: 'Publish Test Report (for non-forks)' | ||
| # The scripts shard runs the root script harness, which has no | ||
| # per-workspace junit.xml output (scripts/tests/vitest.config.ts does | ||
| # not configure the junit reporter). Skip it to avoid a "no test | ||
| # report files found" error. | ||
| # The scripts shard runs the root script harness, which produces no | ||
| # per-workspace junit.xml. Skip it to avoid a "no test report files | ||
| # found" error. | ||
| if: |- | ||
| ${{ always() && matrix.shard != 'scripts' && (github.event.pull_request.head.repo.full_name == github.repository) }} | ||
| uses: 'dorny/test-reporter@a43b3a5f7366b97d083190328d2c652e1a8b6aa2' # ratchet:dorny/test-reporter@v3 | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -187,21 +187,29 @@ Type checking uses `tsc --noEmit`: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bun run typecheck | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #### Bun-backed Test Orchestration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #### The canonical test command | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| The project also provides a Bun-backed root test entry point that preserves | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| the same coverage and setup guarantees as `npm run test` (issue #2463): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| One command runs the complete suite: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bun run test:bun | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| This script (`scripts/test.ts`) orchestrates testing across all workspace | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| packages using Bun as the runtime. Each workspace's tests still run under | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Vitest — not Bun's native test runner — so all Vitest-specific APIs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (`vi.stubEnv`, `vi.unstubAllEnvs`, `vi.mocked`, `vi.setSystemTime`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `it.runIf`, etc.) and per-package `vitest.config.ts` configuration remain | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fully available. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| packages using Bun as the runtime, then runs the script harness. Migrated | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workspaces execute under **Bun's native test runner** via | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `scripts/run_bun_tests.ts` (one isolated process per test file); the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workspaces still finishing their migration run under Vitest, with the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Vitest-compatibility shim (`test-setup/augment-bun-vi.ts`) supplying the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `vi.*` and `it.*` helpers Bun lacks. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Two roots are excluded from that run because they call a real provider and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| consume quota — run them explicitly when you have credentials: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| npm run test:integration:sandbox:none | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| npm run test:all_evals | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+192
to
+211
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Describe Lines 192 and 206 conflict. Proposed wording change-One command runs the complete suite:
+One command runs the complete non-credentialed suite:📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| The script explicitly runs each workspace's `pretest` lifecycle hook before | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| its test phase (npm does this automatically; Bun does not), so the agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <testsuites name="bun test" tests="1" assertions="0" failures="1" skipped="0" time="0.156603"> | ||
| <testsuite name="save_memory.eval.ts" file="save_memory.eval.ts" tests="1" assertions="0" failures="1" skipped="0" time="0" hostname="Andrews-MacBook-Pro.local"> | ||
| <testsuite name="save_memory" file="save_memory.eval.ts" line="14" tests="1" assertions="0" failures="1" skipped="0" time="0.009" hostname="Andrews-MacBook-Pro.local"> | ||
| <testcase name="should be able to save to memory" classname="save_memory" time="0.009433" file="save_memory.eval.ts" line="115" assertions="0"> | ||
| <failure type="AssertionError" /> | ||
| </testcase> | ||
| </testsuite> | ||
| </testsuite> | ||
| </testsuites> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the smoke command with its stated scope.
Line 819 does not pass
--skip-pretest. The surrounding comment says that pretest hooks are skipped and that only orchestrator failures matter. A future failingtest-utilspretest hook would fail this job for a different reason. Add--skip-pretest, or change the comment and job scope.Proposed command change
📝 Committable suggestion
🤖 Prompt for AI Agents