test: run the type-export fixtures through one compile and assert their output - #37812
test: run the type-export fixtures through one compile and assert their output#37812robobun wants to merge 5 commits into
Conversation
…ir output type-export.test.ts built 18 standalone executables, one per fixture, and the skipped "run" test next to each of them split the concurrent batch, so the compiles ran one after another (53s on the slowest CI lane). All 18 fixtures now go into one temp tree and a generated runner imports them one by one, reporting every result keyed by fixture name. The runner is executed from source (still skipped, #7384), from one Bun.build of all the entry points, and as one --compile --bytecode --format=esm --splitting executable, where every fixture is its own chunk with its own module record. The remaining tests assert the exact stdout/stderr/exit code, or the exact error line, instead of stopping at the exit code.
|
Updated 12:39 PM PT - Aug 12th, 2026
✅ @robobun, your commit 3b8e6f632db0e66fa853bcd065657a796933496a passed in 🧪 To try this PR locally: bunx bun-pr 37812That installs a local version of the PR into your bun-37812 --bun |
|
Status: test-only change, CI green on head 3b8e6f6 (build 93258: every lane, including windows x64/aarch64, debian-13-x64-asan with leak checking now enabled for this file, and ubuntu 25.04 x64, the lane that took 53s before). Ready for review. Measured on one machine with the same debug build: |
WalkthroughThe type-export test suite now uses generated fixtures and grouped build or compile execution. It adds normalized diagnostic assertions, decorator and namespace coverage, CommonJS interoperability checks, and table-driven re-export tests. ChangesType export test coverage
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/js/bun/typescript/type-export.test.ts`:
- Around line 364-367: Update the conditional source-location assertion in the
duplicate-declaration test to assert whether the normalized stderr contains the
expected a.<fmt> location, using the entry-point condition as the expected
boolean. Ensure both entry-point cases validate the location and can fail, while
preserving the existing stdout and exit-code assertions.
🪄 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
Run ID: 68ec73ee-4ae8-43d5-819d-0cb7eb8c0cca
📒 Files selected for processing (1)
test/js/bun/typescript/type-export.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It's a well-structured test-only refactor with a thorough coverage mapping and stronger assertions than before. Because it changes how the compile-mode coverage is exercised — 18 standalone executables become one --compile --splitting build where each fixture is a dynamically-imported chunk — a maintainer familiar with the bundler's module_info/bytecode path should confirm that still tests what the original 18 compiles protected.
What was reviewed:
- Traced every test in the old file to its counterpart in the new structure per the description's table; nothing dropped, and the ownkeys/decorator fixtures gain
buildcoverage. - Checked that assertions are strengthened (exact error lines with
<dir>substitution, combined{stdout, stderr, exitCode}equality, decorator now assertsdesign:type === Object) rather than weakened. - Verified the per-fixture
tsconfig.jsonfor the decorator fixture sits where the bundler's per-file tsconfig walk will find it, and that theReflect.metadatamock in the shared runner can't leak into other fixtures. - Confirmed
await using tempDirreplaces the leakedtempDirWithFilesdirs so the ~1 GB compiled executable is cleaned up.
Extended reasoning...
Overview
Single-file test refactor of test/js/bun/typescript/type-export.test.ts (~400 lines rewritten). The 16 b×c re-export fixtures plus the ownkeys and decorator fixtures are collapsed from 18 separate bun build --compile invocations into one temp tree with a generated runner.ts that dynamically imports each fixture and reports results as one JSON object. Three modes (run skipped, build, compile) each become a single test. The remaining error-case tests are converted to table-driven form with exact-error-line assertions via normalizeBunSnapshot. Measured ~10× speedup on debug builds and cleans up ~14 GB of leaked executables per debug run.
Security risks
None. Test-only change; no src/ modifications, no network, no auth/crypto surface. All spawned processes use bunEnv and hermetic tempDir fixtures.
Level of scrutiny
Moderate. It's test-only, but REVIEW.md is emphatic that test refactors must not silently weaken coverage and must still fail for the right reason. The core design change — replacing per-fixture standalone executables with one --compile --splitting build where fixtures are chunks reached via import() — is argued in the description to be stronger coverage (each chunk gets its own module record whose export list must correctly omit type-only names). That argument is plausible and well-reasoned, but confirming it exercises the same module_info generation path the original tests were written to guard is a bundler-internals judgment I'd want a maintainer to make rather than auto-approve.
Other factors
- The bug-hunting pass found nothing. CodeRabbit's one nit (conditional location assertion in
through export merge) was addressed in be2577f by moving expected stderr into the table. - Assertions are uniformly tighter than before:
toIncludeon error fragments → exact normalized error line including the module path; bareexitCodechecks → combined{stdout, stderr, exitCode}equality; decorator test went from "exits 0" to asserting the emitteddesign:typemetadata. - Harness conventions look right:
await using tempDir,bunEnv, concurrent pipe drains,describe.concurrentwith a comment explaining why (the skipped test would otherwise break the concurrent batch), and the 30s local-only compile timeout mirrorsitBundled. - The generated runner monkey-patches
console.logper fixture inside the subprocess and restores it infinally; the decorator fixture'sReflect.metadatamutation is isolated to that subprocess and is the last fixture in insertion order regardless.
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It's a well-documented test-only refactor with a clear coverage-preservation table, but it does change the isolation model (all 18 fixtures now share one process/realm via a generated runner, and --splitting is added to the compile path), so a maintainer familiar with what this file guards for #7384/#8439 should confirm the merged runner still exercises the same module-record paths.
What was reviewed:
- Traced the old→new coverage table; every previous test maps to a fixture or table row, and assertions are uniformly tightened (exact error lines / stdout / exit codes vs. the old
toIncludefragments). - Checked fixture ordering in the shared runner: the decorator fixture's
Reflect.metadatamutation runs last, so it can't leak into the b×c or ownkeys fixtures. - Verified
tempDir/makeTreeSynchandle the nestedfixtures/<b>/<c>/...keys, and thatnormalizeBunSnapshotaccepts theDisposableStringdir for<dir>substitution. - Confirmed the removed 4-slot compile semaphore is safe: only one
--compileremains.
Extended reasoning...
Overview
This PR rewrites test/js/bun/typescript/type-export.test.ts (~400 lines changed, single file, test-only) to collapse 18 separate bun build --compile --bytecode invocations into one, dropping the file's runtime from ~50s to ~6s on debug builds. The 16 b×c re-export/import combinations plus the ownkeys and decorator fixtures are now written into one temp tree and driven by a generated runner.ts that dynamic-imports each fixture and prints a keyed JSON result. The remaining error-case, through-export-merge, commonjs, and re-export-form tests are table-driven with tightened assertions.
Security risks
None. This is a test file that spawns bun against generated fixtures in temp directories; no auth, crypto, network, or untrusted-input handling is touched.
Level of scrutiny
Moderate. It touches no runtime code, so it can't regress user behavior — but it can silently drop or weaken coverage, which the repo review guidelines call out explicitly ("Never silently weaken, skip, or delete an existing test"). I traced the description's before/after table against both versions of the file and found every prior test accounted for, with assertions strengthened rather than relaxed (exact errorLine matches replacing toMatch/toInclude, full {stdout, stderr, exitCode} objects replacing bare exit-code checks, and the decorator fixture now asserting design:type === Object instead of just exit 0). The one CodeRabbit nit (conditional location assertion) was addressed in be2577f by moving the expected stderr into the table.
Other factors
The part that most warrants a maintainer glance is the change in what compile actually exercises: previously each fixture was its own standalone executable with everything inlined into one chunk; now one executable is built with --splitting and each fixture becomes its own chunk with its own module record. The description argues this is stronger (the re-exporting b module now has a real export list to check), which reads correctly to me, but it's a semantic change to the test's shape that someone who owns the module_info work (#7384, #35605) should confirm. Similarly, the decorator fixture's tsconfig.json (for emitDecoratorMetadata) is now discovered per-source-file inside a multi-entry Bun.build / --splitting compile rather than being the sole tsconfig at the build root — the passing design_type: "Object" assertion proves it's found today, but it's the kind of coupling worth a human ack. CI (#93225) is still building at time of review.
…xport.test.ts The file was listed in no-validate-leaksan.txt only because it was slow; with one compile it passes with leak checking in about 9s on a debug build.
…elf-contained A fixture that takes the runner down now shows up as the first missing key instead of emptying the whole report.
1 participant
Problem
test/js/bun/typescript/type-export.test.tsis one of the slowest non-integration test files: 53s on the slowest lane of build 92779, 50s to 61s per local debug run.test.concurrent: every fixture also had a skippedruntest, and a non-concurrent entry closes the batch of concurrent tests that run together.test/no-validate-leaksan.txt, so ASAN lanes did not leak-check it.Fix
buildis oneBun.buildplus one process (was 16 of each),compileis one--compile --splittingexecutable plus one run (was 18),runstays skipped for export not found when executing typescript file #7384.--splittingeach fixture, and eachbit loads withimport(), gets its own chunk, bytecode and module record; before, everything was inlined into one chunk and the record held nothing type-related.toEqualkeyed by fixture name, and a crash appears as the first missing key with the crash in stderr. Assertions also tighten: empty stderr, the decorator fixture prints thedesign:typeit received (Object, per emitDecoratorMetadata fails when interfaces are imported withoutimport type#8439), and error cases compare the whole error line including the module it names.runtoday fails on the 10 fixtures runtime: attach ModuleInfo to ESM transpiles so TypeScript type-only re-exports resolve #35605 is expected to fix.Background
export type, or an interface re-exported by name) has no runtime binding, so a re-export chain that names one must drop it at link time or the linker reportsexport 'my_string' not found. This file checks that acrossrequire,import *,import()and named imports.--compile --bytecode --format=esmoutput, not from the runtime transpiler (export not found when executing typescript file #7384), which is whyrunis skipped and onlybuildandcompileare asserted.bun build --compilemakes a standalone executable by copying the whole bun binary and appending the bundle, so a compile costs the copy, not the bundling.--splittingputs eachimport()target in its own chunk instead of inlining it.bun:test,test.concurrenttests only run together within a batch, and any non-concurrent test in between (a skipped one included) ends the batch;describe.concurrentkeeps a group in one batch.test/no-validate-leaksan.txtlists test files the ASAN lanes run without LeakSanitizer's exit check; removing a file turns that check back on for it.no test proof · iteration 0 · Platform-specific test-only change; deferring to CI.
Original description
test/js/bun/typescript/type-export.test.tsis one of the slowest non-integration test files (53s on the slowest lane of build 92779, a release x64 lane). Test-only change (the test file plus its line intest/no-validate-leaksan.txt); nosrc/changes.Why it was slow
Every one of the 16
bxcfixtures, plus the ownkeys and decorator fixtures, ranbun build --compile --bytecode --format=esmon its own: 18 executables, each one a full copy of the bun binary (about 100 MB release, about 800 MB debug). The file already usedtest.concurrent, but each fixture's skippedruntest sits in a plaindescribe, so it is a non-concurrent entry, and a non-concurrent entry closes the batch of concurrent tests that run together (append_or_extend_concurrent_groupinsrc/runtime/test_runner/Order.rs). The compiles therefore ran one after another: 18 x about 3s.What changed
fixtures/<name>/...) and a generatedrunner.tsimports them one after another, capturing what each prints and printing one JSON object keyed by fixture name. The three modes become three tests in onedescribe.concurrent:run:bun runner.ts, still skipped (export not found when executing typescript file #7384). If it is un-skipped today it fails with the 10 affected fixtures (the 8export-from/import-then-exportcombinations, ownkeys, decorator) each reportingSyntaxError: export 'my_string' not found in './a.ts'etc. under their own key, so it is ready for runtime: attach ModuleInfo to ESM transpiles so TypeScript type-only re-exports resolve #35605 to flip.build: oneBun.buildof all the entry points (without splitting each entry still gets its own self-contained bundle; the per-fixture module code is byte-identical to a build of that fixture alone, only the shared helper preamble differs), then onebun runner.tsover the outputs. Was 16 builds + 16 processes.compile: onebun build --compile --bytecode --format=esm --splitting runner.ts, then one run of the executable. With--splittingevery fixture is its own chunk with its own bytecode and module record, and in theawait import("./b")fixturesbbecomes a chunk of its own, so its module record's export list (my_value,my_only, and not the type-onlymy_string) is now exercised by the compiled build; previously everything was inlined into one chunk per executable and the record had nothing type-related in it. Was 18 executables.[name, result]line per fixture and the assertion is a singletoEqualagainst{ results: { "<fixture>": ... }, stderr: "", exitCode: 0 }, so the diff names the fixture and shows its parsed output, or{ error, lines }with whatever it managed to print before failing. A fixture that takes the process down shows up as the first missing key, with the crash instderr.--splittingand the extra chunks add about 0.1s), so the file's exposure to the default per-test timeout goes from 18 tests to 1.test/no-validate-leaksan.txtlisted this file under# Slow. With one compile that reason is gone, so the entry is removed and ASAN lanes validate leaks for it again; locally,bun bd testwith the CI runner's LeakSanitizer setup (BUN_DESTRUCT_VM_ON_EXIT=1,detect_leaks=1,test/leaksan.supp, its--timeout) passes at this head in 10.7s on a loaded machine (load average about 75) with no leak reports and nothing left in the temp dir.await using, so the executable is deleted afterwards. Before, the 18 executables were left in the temp dir (about 14 GB per debug run).Every shape the file covered is still covered:
re-export with <4 b> > import with <4 c> > run / compile / build<b>/<c>fixtures, in therun/build/compiletestscheck ownkeys from a star import > run / compileownkeys-of-star-importfixture (now also covered bybuild)import only used in decorator (#8439) > run / compileimport-only-used-in-decoratorfixture (now also covered bybuild)import not found > none / default with same name / type,js file type import,... with default export,js file with through export,... 2,check mergeimporting a name that is not exported as a valuetable, same namesjs file type exportthrough export merge(js/ts x 4 x main/a)check commonjsexport * from,export * as ns from(js/ts x main/a),export type {Type} from(ts x main/a)re-export formstableAssertions
Reflect.metadatarecorder for the duration of its class definitions and prints thedesign:typeit received, so it asserts theObjectthat tsc emits for an interface (the expected behaviour in emitDecoratorMetadata fails when interfaces are imported withoutimport type#8439) instead of just exiting 0.export */export * as ns/export type {}: exact stdout per file (b,test, or nothing for the re-exporting file itself) plus empty stderr, instead of a bare exit code.'<dir>/a.ts','<dir>/ts.ts','./ts.ts'), plus empty stdout and exit code 1, instead oftoIncludeon a fragment.js file type exportsnapshots the whole normalized stderr, including theat <dir>/a.js:1:9location.through export mergekeeps JSC's message for themainentry and, for theaentry, expects bun's message immediately followed by itsat <dir>/a.<ext>:1:location (themainexpectation is the line parser: .cjs/"type":"commonjs" rejects ESM export and top-level await #33899 will change).Timing
Same machine, same debug build,
bun bd test test/js/bun/typescript/type-export.test.ts:USE_SYSTEM_BUN=1 bun test)Five further
bun bd testruns while the host was heavily loaded (load average 60 to 120) took 8.3s to 11.0s and passed every time; the same load is what pushed a single debug compile past 5s and motivated the ceiling. Test count goes from 70 pass + 18 skip to 38 pass + 1 skip, per the table above;expect()calls from 196 to 57, with the 18 fixtures compared in twotoEquals per mode and the 16through export mergeruns in two each.