Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review. WalkthroughChangesCoverage bail-out reporting
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change adds coverage output when test execution stops because of --bail, while preserving existing exit behavior; targeted debug, release, and regression checks pass, and no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description clearly explains the problem, implementation, behavior, scope, and verification steps. It does not use the template headings exactly, but it provides the required information in equivalent sections. Comment |
|
Status: ready for review. Rebased onto main at Reproduced on the current release with After the latest rebase (past #40678) the change is: CI on build 107281: 179 of 181 jobs pass, including every test this PR touches. The two red jobs are unrelated to this change and also fail on main: the |
|
Updated 10:03 PM PT - Aug 27th, 2026
❌ @robobun, your commit a082856 has 2 failures in
The baseline build contains instructions not available on Static scan violations
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — focused fix that routes both serial --bail exits through a shared bail_out() helper which now writes coverage before the summary.
What was reviewed:
- Verified the extracted 8-way
<TEXT, LCOV, COLORS>dispatch inwrite_code_coverage_if_neededis arm-for-arm identical to the inlined version it replaces, and the normal end-of-run path still passes its owncoverage_optionsso thefractions.failingexit-code check at line ~3041 is unchanged. - Confirmed
generate_code_coverage/print_code_coverageonly touchvm.global()(which takes&self), so the&mut → &relaxation is sound;VirtualMachine::get()at the mid-file bail site returns&'static. - Checked the two new tests cover both bail sites (test failure and top-level throw), assert the text table and
lcov.infovia inline snapshots, and follow harness conventions (tempDir,normalizeBunSnapshot, concurrent, exit code asserted last).
Extended reasoning...
Overview
This PR fixes bun test --bail --coverage skipping coverage output when bail trips. It touches two files: src/runtime/cli/test_command.rs (extract bail_out() and write_code_coverage_if_needed() helpers, relax &mut VirtualMachine to &VirtualMachine on two coverage functions) and test/cli/test/coverage.test.ts (two new regression tests).
Security risks
None. This is CLI reporting/output code for the test runner — no untrusted input parsing, no auth, no network, no crypto.
Level of scrutiny
Medium-low. The change is primarily a deduplication refactor of two identical bail-out sequences into one helper, plus inserting a call to the existing coverage-generation code before print_summary(). The 8-way const-generic dispatch was moved verbatim (I checked each arm's tuple → generic-param mapping matches the original). The normal end-of-run path is behavior-preserving: it still calls the same dispatch, still propagates errors with ?, and still uses the local coverage_options whose fractions.failing feeds the exit-code decision. On bail, a coverage-write error is printed via Output::err and swallowed, which is correct since the process is already exiting non-zero.
Other factors
- The
&mut VirtualMachine → &VirtualMachinerelaxation is safe: both functions only callvm.global(), which isfn global(&self) -> &'static JSGlobalObject.render_lcov(unchanged, still&mut) is not on the bail path. - The load-failure bail site now gains an
Output::flush()after the "Bailed out" line (previously onlyhandle_test_completedhad it) — a harmless consistency improvement. - Tests are well-constructed: one per bail site, inline snapshots on both the stderr table and the lcov file contents,
describe.concurrentfor independent subprocess spawns,coverageSkipTestFiles = trueso onlymod.tsappears in the report, and the fixture deliberately uses a two-line uncovered branch to sidestep the pre-existing single-line-range display quirk noted in the PR description. - No CODEOWNERS coverage on the touched paths. No outstanding human review comments.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — extracts the two duplicated serial bail-exit sequences into CommandLineReporter::bail_out and adds coverage writing to it, so --bail --coverage now emits the text table and lcov file for what ran before exiting.
What was reviewed:
- The 8-way
<TEXT, LCOV, COLORS>dispatch moved verbatim intowrite_code_coverage_if_needed; the normalexecpath still passes its own&mut coverage_optionsso the downstreamfractions.failingexit-code check at :3040 is unchanged. &mut VirtualMachine→&VirtualMachineongenerate_code_coverage/print_code_coverage: both only callvm.global(), and the mid-file bail site usesVirtualMachine::get() -> &'static.- Both bail sites (
handle_test_completedandrun_one_file) now share one code path; the load-failure site gains anOutput::flush()it previously lacked. - New tests cover one case per bail site, assert exact stderr +
cov/lcov.infosnapshots and exit code 1;detect_leaks=0on the spawned child is scoped and cites #32183 (pre-existing bail-exit leak, not introduced here).
Extended reasoning...
Overview
Two files: src/runtime/cli/test_command.rs (~50 net lines, mostly moved) and test/cli/test/coverage.test.ts (+162 lines, two new tests). The Rust change extracts the identical print-summary → "Bailed out" → JUnit → timings sequence from the two serial --bail exit sites into CommandLineReporter::bail_out(vm), and inserts a coverage write (write_code_coverage_if_needed) before the summary. The 8-arm const-generic dispatch that was inlined in TestCommand::exec is lifted unchanged into the new helper, and exec now calls it too (still guarded by !ran_parallel). generate_code_coverage / print_code_coverage are relaxed from &mut VirtualMachine to &VirtualMachine since they only read vm.global().
Security risks
None. This is CLI test-runner reporting plumbing — no parsing of untrusted input, no network, no auth/crypto. The lcov writer already existed and is unchanged; only when it is invoked changed.
Level of scrutiny
Medium-low. The refactor is mechanical: two copy-pasted blocks collapsed into one helper (which REVIEW.md requires anyway), plus one new call added to that helper. I verified the moved dispatch is byte-for-byte identical to what was in exec, that the normal path still writes into the same coverage_options local so the fractions.failing threshold check at :3040 still fires, and that CodeCoverageOptions derives Clone so bail_out's clone compiles. The &mut → & relaxation is safe: VirtualMachine::get() returns &'static VirtualMachine, and both callers of vm inside the coverage path only invoke vm.global(). In bail_out, a coverage write error is printed via Output::err rather than propagated — appropriate since the process is about to exit(1) anyway; the normal path still uses ?.
Other factors
- Tests are strong: one per bail site, exact inline snapshots for both stderr (text table between "(fail)" and "Bailed out") and the lcov file contents, plus
exitCode === 1. The fixture uses a two-line uncovered branch to avoid depending on the pre-existing single-line-range display quirk mentioned in the description. - The
detect_leaks=0on the spawned child is narrowly scoped, composed with any existingASAN_OPTIONS, and commented with the tracking issue (#32183); the PR description documents that a clean-exit--coveragerun underBUN_DESTRUCT_VM_ON_EXIT=1is LSan-clean, so the coverage code itself is not the leak source. - The comment-cop bot's inline note was addressed (doc comment trimmed to one line) and the thread is resolved.
- The load-failure bail site (
run_one_file) previously lacked anOutput::flush()after the "Bailed out" message;bail_outadds it, which is a harmless improvement (the other site already had it). - Evidence gate shows the two new tests fail on the debug build without the fix and pass with it; existing bail/JUnit/parallel-coverage tests were re-run per the description.
f229f68 to
daabca0
Compare
There was a problem hiding this comment.
LGTM — focused fix that routes both serial --bail exits through a shared bail_out() helper so coverage reporters run alongside the JUnit/timings hooks that already did.
What was reviewed:
- Verified
generate_code_coverage/print_code_coverageonly callvm.global(), so the&mut VirtualMachine→&VirtualMachinerelaxation is sound;handle_test_completedcorrectly usesVirtualMachine::get()where novmis in scope. - Confirmed the normal end-of-run path still passes its own
&mut coverage_optionssofractions.failingpropagates to the exit-code check attest_command.rs:3034; on bail the cloned options don't matter since the process exits 1 regardless. - Tests cover both bail sites (mid-file test failure and load-time throw), assert both the text table and
lcov.infocontents via inline snapshots, and follow harness conventions (describe.concurrent,tempDir,detect_leaks=0scoped to the #32183 exit path with a comment).
Extended reasoning...
Overview
This PR fixes bun test --bail --coverage to emit coverage reports (text table and lcov.info) when the run stops early due to --bail. Previously, the two serial bail exits (CommandLineReporter::handle_test_completed and TestCommand::run_one_file) called print_summary → JUnit → timings → exit, skipping the coverage-generation call that only lived on the normal end-of-run path in TestCommand::exec.
The fix extracts a CommandLineReporter::bail_out(vm) helper that both sites now call, and a write_code_coverage_if_needed(vm, opts) helper holding the enabled-check + reporter dispatch that was previously inlined in exec. The normal path calls the same helper (still guarded by !ran_parallel). generate_code_coverage and print_code_coverage are relaxed from &mut VirtualMachine to &VirtualMachine because they only call vm.global() (verified at lines 1683 and 1934), and the mid-file bail site only has VirtualMachine::get() -> &'static VirtualMachine available.
Security risks
None. This is CLI-local report generation on the test runner's exit path — no network, no user-supplied paths beyond what --coverage-dir already accepts on the normal path, no auth/crypto. The lcov write reuses the existing temp-file-and-rename path in print_code_coverage.
Level of scrutiny
Medium. The change touches test_command.rs (a large file) but is a targeted refactor of two duplicated exit sequences into a shared helper plus one added call. The refactor is behavior-preserving for the existing bail outputs (summary, "Bailed out" message, JUnit, timings — same order, same flush points plus one leading Output::flush() before coverage) and adds only the coverage step. I checked that:
- The
&mut → &VM change is safe: both functions only readvm.global(). - The normal path's
coverageThresholdexit-code semantics are preserved:execstill passes its own&mut coverage_options, soopts.fractions.failingset insideprint_code_coverage(line 2031) still reaches the check at line 3034. On bail, the helper clones a freshCodeCoverageOptionsfromself.jest.test_options.coverage, sofractions.failingdoesn't propagate — but bail always exits 1 anyway, so that's immaterial. - Coverage-write errors on bail are printed via
Output::errrather than propagated (the process is about to exit 1); the normal path still uses?.
Other factors
- Tests are thorough: two new tests in
coverage.test.ts, one per bail site, each asserting the full stderr snapshot (table present, positioned before the summary/"Bailed out" line) and the exactlcov.infobytes. The fixture deliberately uses a two-line uncovered branch to sidestep an unrelated single-line-range display bug. - The
ASAN_OPTIONS: detect_leaks=0override is scoped to the spawned child, composed with any existingbunEnv.ASAN_OPTIONS, and justified inline with a link to #32183 (mid-file bail exits viaGlobal::exitwithout VM teardown — a pre-existing leak unrelated to coverage). - The comment-cop bot's feedback was addressed (doc comment trimmed to one line in a follow-up commit; thread resolved).
- The PR description documents cross-verification against the JUnit-on-bail regression tests (26851, 12250), the existing bail tests in
bun-test.test.tsandtest-test.test.ts, and the parallel coverage/bail path. - The change matches the existing
write_junit_report_if_needed/write_timings_if_neededpattern exactly — coverage joins the same set of "must also run on bail" hooks.
daabca0 to
b1c248a
Compare
There was a problem hiding this comment.
LGTM — focused fix that routes both serial --bail exits through a shared bail_out helper so coverage reports are written alongside the JUnit and timings files that already were.
What was reviewed:
- Confirmed the normal
execpath still writes to the caller's&mut coverage_options, so thefractions.failing→ exit-code check at the end ofexecis preserved. - Checked the
&mut VirtualMachine→&VirtualMachinerelaxation:generate_code_coverage/print_code_coverageonly readvm.global(), and therun_one_filebail site reborrowsvmmutably again afterbail_outreturns. - Verified
CodeCoverageOptions: Clone, and that the discardedfractions.failingon the bail-path clone is irrelevant since bail always exits 1. - Tests follow harness conventions (
tempDir, concurrent stderr/exited drain, inline snapshots, exit code asserted last); the two-line uncovered branch anddetect_leaks=0are both justified in the PR body.
Extended reasoning...
Overview
The PR fixes bun test --bail --coverage skipping coverage output when bail trips. It touches two files: src/runtime/cli/test_command.rs (net ~+38/-32) and test/cli/test/coverage.test.ts (+164). The Rust change adds CommandLineReporter::bail_out (consolidating the two duplicated serial bail sequences and inserting a coverage write) and write_code_coverage_if_needed (extracting the enabled gate + reporter-flag plumbing that was inlined in exec). generate_code_coverage / print_code_coverage now take &VirtualMachine instead of &mut, which lets the mid-file bail site (which only has VirtualMachine::get() -> &'static VirtualMachine) call them.
Security risks
None. This is CLI test-runner reporting logic — no untrusted input parsing, no auth/crypto/permissions, no network. The only I/O added on the bail path is the same coverage-file write that already ran on the normal path.
Level of scrutiny
Medium-low. The change is a mechanical dedup of two copy-pasted exit sequences plus one new call inserted into that sequence, mirroring write_junit_report_if_needed / write_timings_if_needed which were already there. I verified the refactor is behavior-preserving on the non-bail path: the !ran_parallel guard is kept, opts.enabled moved inside the helper, &mut coverage_options is still the exec-scope local so the coverage_options.fractions.failing && coverage_options.fail_on_low_coverage exit-code check downstream still sees the mutation, and errors still propagate with ?. On the bail path the coverage-write error is printed with Output::err instead of propagated, which is correct since the process is about to Global::exit(1) regardless. The &mut → & relaxation is safe: I read the bodies and they only call vm.global(); render_lcov (the parallel-coordinator path) keeps &mut and is untouched.
Other factors
- The two new tests cover both bail sites (mid-file test failure and top-level load failure), assert both the stderr text table and the on-disk
lcov.infobytes via inline snapshots, and were shown in the PR evidence to fail on both release and ASAN main and pass on the PR build. - The
detect_leaks=0on the spawned child is scoped to a documented pre-existing leak (#32183) in theGlobal::exit(1)bail path itself, not in the coverage code being added; the PR description records that a passing--coveragerun underBUN_DESTRUCT_VM_ON_EXIT=1is LSan-clean. - The one bot comment (paragraph-long doc comment) was addressed and the thread is resolved; the current diff has one-line doc comments on both new helpers.
- No prior claude[bot] reviews and no outstanding human reviewer comments.
Both serial bail exits printed the summary and wrote the JUnit report and timings, but the coverage table and lcov.info were only produced on the normal end-of-run path. Route both bail sites through one helper that emits the configured coverage reporters for the files that did execute before exiting.
A mid-file bail exits through Global::exit without VM teardown, so under detect_leaks=1 the child appends a LeakSanitizer report to stderr and aborts (#32183). The stderr snapshot is about what bail writes, so run the child with detect_leaks=0 like other tests that spawn into a known leaky exit path.
b1c248a to
16a58f4
Compare
generate_code_coverage returns the lcov write error instead of exiting, so the normal end-of-run path keeps its print-and-exit behavior while bail_out prints the error and still writes the summary, JUnit report and timings.
There was a problem hiding this comment.
LGTM — the lcov-write-failure regression I flagged is now fixed: generate_code_coverage returns bun_sys::Result<()> instead of exiting internally, bail_out logs the error and continues to write the summary/bail line/JUnit/timings, and the new "an lcov write failure does not stop the other reports" test pins that behavior. The normal end-of-run path in exec still exits 1 on the same error, so no behavior change there.
Extended reasoning...
Overview
This PR makes bun test --bail --coverage emit coverage reports (text table + lcov) before exiting. Two duplicated bail sequences in src/runtime/cli/test_command.rs — the per-test-failure path in handle_test_completed and the file-load-error path in run_one_file — are consolidated into a new CommandLineReporter::bail_out(&mut self, vm: &VirtualMachine) that runs coverage first, then the summary, "Bailed out" line, JUnit, and timings. generate_code_coverage and for_each_coverage_report are relaxed from &mut VirtualMachine to &VirtualMachine (they only read vm.global()), and generate_code_coverage now returns bun_sys::Result<()> so callers decide whether an lcov write failure terminates the process.
Security risks
None. This is CLI output ordering/end-of-run reporting for the test runner. No auth, crypto, network, or untrusted-input parsing is touched.
Level of scrutiny
Moderate. The one real risk was the error-handling refactor: previously generate_code_coverage called Global::exit(1) internally on an lcov write failure, so inserting it ahead of print_summary/JUnit/timings in bail_out would have regressed those outputs on a bad --coverage-dir. My prior review flagged exactly that; the latest commit fixes it by hoisting the exit decision to the callers — bail_out logs and continues (process exits 1 anyway), while the normal exec path still Global::exit(1)s, byte-identical to base. I verified the exec call site at line 2500-2503 and confirmed coverage_options.fractions.failing is still mutated on the same &mut coverage_options there, so the coverage-threshold exit-code check downstream is unchanged. In bail_out the options are cloned, so threshold mutation on the clone is discarded — irrelevant since bail always exits 1.
Other factors
The change follows REVIEW.md's "deduplicate within your own diff" and "fix the whole class" (both serial bail sites now share one helper; --parallel was already correct per the PR description). Tests are appended to the existing test/cli/test/coverage.test.ts in a describe.concurrent block, use tempDir/bunExe/spread bunEnv/await using/Promise.all for pipe draining, assert stderr before exitCode, and cover all three paths: test-failure bail, load-error bail, and lcov-write-failure-during-bail. The detect_leaks=0 env override is scoped and commented with the tracking issue (#32183). No outstanding CHANGES_REQUESTED reviews on the timeline.
…he full report (#41423) ### Problem - `test/regression/issue/26851.test.ts` takes up to 12s on the debian 13 x64-asan lane. It is in the slowest 5 percent of test files. Both tests spawn a `bun test --bail` child, and they ran one after the other. - On that lane each child exits 134, not 1. `bun test --bail` exits with a bare `exit(1)`, so the `BUN_DESTRUCT_VM_ON_EXIT` teardown is skipped and LeakSanitizer aborts the child after several seconds of report symbolization (#32183, fix open in #39010). Locally each child takes 0.3s. Under the lane environment it takes 3.8s. The old `expect(exitCode).not.toBe(0)` hid the abort. - The old assertions only checked that the JUnit file exists and contains four substrings. They did not check the counters, the failure element, the bail message, or the exit code. ### Fix - Add the file to `test/no-validate-leaksan.txt`, next to `test/regression/issue/12250.test.ts`, which is on the list for the same bail exit. The entry is to be removed when #39010 lands. Without LeakSanitizer the children exit 1 in well under a second. - Run both tests with `test.concurrent`. Each test owns its tempDir and its outfile, so the two children overlap. A shared `runBailWithJUnit` helper does the spawn and reads the report. - Assert the full JUnit document with `toMatchInlineSnapshot`: the `testsuites` root with `tests`, `assertions`, `failures` and `skipped`, one `testsuite` per file that ran, the `testcase` names and lines, and the `failure` element with its type and message. Only the `time` and `hostname` attributes are normalized. Assert the child output before the exit code: stdout is the version banner, stderr has the `(pass)`/`(fail)` lines, the `Ran N tests across M files.` summary and `Bailed out after 1 failure`. The exit code is exactly 1. - Add a second test after the failing one, and a third test file `c_never.test.ts`. Neither appears in stderr or in the report. This proves that `--bail` stopped the run and that the report holds only what ran. Discovery order of the root directory is sorted by name, so `a_pass` runs before `b_fail`. - Verified: `bun bd test test/regression/issue/26851.test.ts`. Plain, 3 runs: before 2.66s to 3.14s wall, after 2.27s to 2.44s wall. With the ASAN lane environment (`BUN_DESTRUCT_VM_ON_EXIT=1`, `detect_leaks=1`): before 9.53s with both children exiting 134, after the list entry that environment is not applied. Also passes with `USE_SYSTEM_BUN=1`. ### Background - `--bail` makes `bun test` stop after the first failing test. #26851 was about the JUnit outfile not being written in that case. - `test/no-validate-leaksan.txt` lists test files for which `scripts/runner.node.mjs` does not set `BUN_DESTRUCT_VM_ON_EXIT` and `detect_leaks=1` on the ASAN lane. - #33704 also adds `test.concurrent` to this file as part of a broad speed pass. This PR is scoped to the one file and also strengthens the assertions. <details><summary>Notes</summary> The first CI run (build 110365) failed on the x64-asan lane with exit code 134 on both tests and 8.7s per test. That is the LeakSanitizer abort described above. The fix for the bail exit itself is in #39010 and out of scope here. The stderr checks use `toContain` rather than a snapshot of the whole stream. Open PRs #39010, #39276, #38974 and #38265 change parts of the bail and reporter output, and a whole-stream snapshot would conflict with them for no gain. The XML snapshot is the thing under test. The failure body in the XML contains `at fail.test.ts:2:40`. The fixture sources are built from fixed strings, not indented template literals, so the column is stable. `test/js/junit-reporter/junit.test.js` snapshots the same shape on every platform. </details> <!-- robobun:evidence:begin --> --- **[auto-merge]** gate passed · iteration 1 · 2 files touched <details><summary>passes on PR (with fix)</summary> ```console Test-only change. Debug/ASAN (expected pass): $ bun bd test 'test/regression/issue/26851.test.ts' $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "test/regression/issue/26851.test.ts" bun test v1.4.3 (e0a2b82) test/regression/issue/26851.test.ts: (pass) --bail writes JUnit reporter outfile [356.62ms] (pass) --bail writes JUnit reporter outfile with multiple files [349.11ms] 2 pass 0 fail 2 snapshots, 15 expect() calls Ran 2 tests across 1 file. [2.42s] Exit: 0 ``` </details> <details><summary>diff hotspot</summary> ``` test/no-validate-leaksan.txt | 3 + test/regression/issue/26851.test.ts | 140 +++++++++++++++++++++--------------- 2 files changed, 87 insertions(+), 56 deletions(-) ``` </details> **gate history** · 3 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests test/no-validate-leaksan.txt 1 1 25 test/regression/issue/26851.test.ts 3 5 19 ``` </details> **root cause** · written by the author bot The regression test for issue #26851 was slow because each of its five tests spawned a separate `bun test` child one after another, and under ASAN every child paid several seconds of startup, while the assertions only checked that the JUnit outfile existed. The fix runs the independent cases concurrently through a shared runner that captures stdout, stderr, the exit code and the report once per fixture set, then asserts the full normalized JUnit document shape, the bail message, the summary line and the exact exit code. A follow-up strips the CI environment variables that make the reporter … <!-- robobun:evidence:end -->
This branch has not been deployed
2 participants
Problem
bun test --bail --coverageemits no coverage at all once bail trips: no text table, nolcov.info, the coverage directory is not even created. The JUnit report and--update-timingsfile for the same run are written.TestCommand::exec,src/runtime/cli/test_command.rs, thegenerate_code_coveragecall before the pass/fail counts). The two serial bail exits skip that path entirely:CommandLineReporter::handle_test_completed(a test failure reaches--bail):print_summary, "Bailed out", JUnit, timings,Global::exit(1).TestCommand::run_one_file(a test file fails to load, e.g. a top-level throw or a syntax error): same sequence, thenglobal_exit().--parallelis not affected:Coordinator::bail_outonly stops dispatching files, and the coordinator still merges the workers' coverage chunks after the run loop ends.jest --bailwrites coverage for what ran.Fix
CommandLineReporter::bail_out(vm), which is what both serial bail sites now call: flush, coverage reporters, summary line, "Bailed out" message, JUnit, timings. Each caller still exits the way it did before (Global::exit(1)mid-file, VM teardown plusglobal_exit()on load failure), so the only behavioral change is the coverage output.bail_outcalls the existinggenerate_code_coverage(vm, opts)on a clone of the configuredCodeCoverageOptionswhen coverage is enabled. The normal end-of-run path inexecstill calls it the same way, so itsfractions.failingexit-code check is unchanged.generate_code_coveragenow returns thelcov.infowrite error instead of exiting inside:execkeeps the old print-and-exit-1 behavior, whilebail_outprints the error and still writes the summary, the "Bailed out" message, the JUnit report and timings (the process exits 1 right after either way).generate_code_coverage/for_each_coverage_reportnow take&VirtualMachine: they only readvm.global(), and the mid-file bail site has no&mut VirtualMachinein scope (it usesVirtualMachine::get()).ByteRangeMappingtable for whatever files were loaded so far; nothing about it depends on the run having finished. The table/lcov produced on bail are byte-identical to what a non-bail run of the same executed code produces (checked on both this build and the release binary).test/cli/test/coverage.test.ts, new--bailblock: one test per bail site, plus one where--coverage-dirpoints at a plain file so the lcov write fails and the JUnit report and bail message must still appear. The first two fail on the current release (table missing from stderr,cov/lcov.infoabsent) and pass with this change. The inline lcov snapshot matches what the release binary produces for the same executed code without--bail.detect_leaks=0(same pattern as other tests that spawn into a known leaky exit): the mid-file bail exits throughGlobal::exitwithout VM teardown, so on the ASAN lane LSan appends a report to stderr and aborts. That is bun test exits after failures are not LSan-clean (bail and failing runs abort under detect_leaks=1) #32183 and happens without--coveragetoo (checked locally:bun test --bailalone reports the same leak set underdetect_leaks=1); a passing--coveragerun underBUN_DESTRUCT_VM_ON_EXIT=1is LSan-clean, so the coverage code itself is not leaking.bun bd test test/cli/test/coverage.test.ts test/regression/issue/26851.test.ts test/regression/issue/12250.test.ts(26851 covers JUnit-on-bail through the refactored sites).bun bd test test/cli/test/bun-test.test.ts -t bail,test/js/bun/test/test-test.test.ts -t bail(the latter exercises the load-failure bail site),test/cli/test/parallel.test.ts -t "coverage|bail".coverageThresholdexit codes unchanged (text reporter exits 1, lcov-only still exits 0 as before; that gap is bun test --coverage-reporter=lcov skips coverage threshold check #32118).Background
--bail[=N]makesbun teststop after N test failures. A test file that throws while being loaded counts as one failure.--coveragemakes the transpiler register every loaded (non-test, by default) file in aByteRangeMappingtable and turns on JSC's control-flow profiler. A coverage report for a file is produced on demand by asking the profiler which basic blocks and functions of that file executed;--coverage-reporter=textprints the table to stderr,--coverage-reporter=lcovwrites<coverage-dir>/lcov.info(via a temp file and rename).CommandLineReporteris the single object that owns run-wide state forbun test(counts, JUnit buffer, timings);write_junit_report_if_neededandwrite_timings_if_neededwere already the "must also run on bail" hooks, this adds coverage to that set.Notes
Rebased onto main after #39770, which turned the
generate_code_coverage/print_code_coverageconst generics into plainboolparameters. The conflict was in the helper and at theexeccall site:write_code_coverage_if_needednow forwardsopts.reporters.text,opts.reporters.lcovandOutput::enable_ansi_colors_stderr()to the new signature instead of holding the old 8-way dispatch, and theexecblock that did the same inline is replaced by the helper call as before. The&mut VirtualMachineto&VirtualMachinechange and both bail sites applied unchanged. Re-ran the suites listed under Verified on the rebased build.Rebased again after #39934, which appended a
--parallel merges line coverage across workerstest to the end oftest/cli/test/coverage.test.ts, the same spot where the--bailblock is added here. Kept both, main's test first. No source conflict. Re-ran the same suites.Rebased a third time after #40678, which replaced the old
generate_code_coverage/print_code_coverage/render_lcovtrio withfor_each_coverage_reportplus agenerate_code_coverage(vm, opts)that reads the reporter flags fromoptsand reports lcov write errors itself. That made thewrite_code_coverage_if_neededhelper from earlier revisions redundant, so it is gone:bail_outnow callsgenerate_code_coveragedirectly behind anenabledcheck, and theexeccall site is left exactly as main has it. The&mut VirtualMachineto&VirtualMachinerelaxation moved to the two new functions. The test file conflict was again a test appended at the end by main (#40586's--parallel merges function coverage across workers); kept both, main's first. Re-ran the same suites.[review] gate passed · iteration 2 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 5 passed · 0 rejected · iteration 2
evidence per changed file