Skip to content

bun:test: don't abort when formatting a test.each title value throws - #36911

Merged
Jarred-Sumner merged 5 commits into
mainfrom
farm/95c1b405/test-each-label-format-panic
Aug 4, 2026
Merged

bun:test: don't abort when formatting a test.each title value throws#36911
Jarred-Sumner merged 5 commits into
mainfrom
farm/95c1b405/test-each-label-format-panic

bun:test: surface throwing custom formatter in test.each titles as a …

648ea0d
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 4, 2026 in 22m 17s

Code review found 2 potential issues

Found 2 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/jsc/ConsoleObject.rs:1813-1839 Same-class panic left unfixed at sibling write!-into-Vec sites
🟡 Nit test/cli/test/bun-test.test.ts:1221-1234 Test does not exercise the %p code path it claims to cover

Annotations

Check warning on line 1839 in src/jsc/ConsoleObject.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Same-class panic left unfixed at sibling write!-into-Vec sites

The same `write!`-into-infallible-sink panic this PR fixes remains at two sibling sites: `src/runtime/test_runner/expect.rs:2801` (`ExpectMatcherUtils::print_value`, reachable via `this.utils.printReceived/printExpected/stringify` in custom matchers — same crate, already returns `JsResult`, one-line swap to `write_to`) and `src/runtime/api/BunObject.rs:723` (`bun_inspect`, where the `.is_err()` branch is dead code because `write_fmt` panics before returning). Per REVIEW.md's whole-class rule, ei

Check warning on line 1234 in test/cli/test/bun-test.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Test does not exercise the %p code path it claims to cover

The second `test.each` (the `%p` case) is never reached: `format_label`'s error propagates via `?` out of the `#[bun_jsc::host_fn]` at `ScopeFunctions.rs:229/251`, throwing synchronously during module evaluation, so the first `$a.b` case aborts the file before the `%p` line runs. The single `toContain("boom from inspect.custom")` is satisfied by the `$path` case alone — reverting the `%p` fix at `jest.rs:792` back to `write!(...).unwrap()` would not break this test. Pass an array of two input st