[robobun]

Related to #24347

Problem

• A test or hook that times out before done is called prints remove the last parameter from the test callback function (or hook). done is often another parameter.
• test("x", (a, b) => {}) gets done in a. test.each([[1, 2]])("x", (a, b, c, d) => {}) gets it in c. Without the last parameter, the line prints again.
• Cause: two literals in print_test_line (src/runtime/cli/test_command.rs:2032, :2040).

Fix

• Both lines now end with remove the parameters that the test callback function gets no value for (hook in the hook line).
• Correct: the call passes only done, after the values that .each binds (src/runtime/test_runner/bun_test.rs:1379).
• Verified: test/js/bun/test/bun_test.test.ts (2 of 8 tests fail on main). Also ran jest-each.test.ts, jest-hooks.test.ts and node-test.test.ts.
• Self-reviewed: 10 concerns raised, 9 addressed. The other applies only without the hook line.

Background

• A bun:test callback with more parameters than values gets done, and Bun waits for its call. test.each passes a row's values first.
• Considered a sentence that says where done is: no command, and wrong for a node:test hook (context, done).
• Also considered: no hint (as Jest), and row counts for .each (new state, wrong after bun:test: support optional trailing tuple elements in test.each #31618 or bun:test: spread test.each rows only if every row is an array, as Jest does #48440).

Downsides

• A tool that matches the old hint text no longer matches.
• For a .each row shorter than another row (test.each does not support tuples with optional elements at the end #24347), the command removes a parameter that other rows use.
• Release build (--lto=off): text +64 bytes, stripped file size equal.

Notes

Other open PRs that touch these two lines

This PR changes only the command at the end of both lines.

• bun test: name the hook kind and its timeout when a hook times out #42365, bun test: emit one anchored GitHub Actions annotation per failed test #41876 and bun test: identify failed hooks and their files #43843 change the start of the hook line (bun test reports a failing beforeAll as "a beforeEach/afterEach hook timed out" #42361) and keep the old command. The one that merges after this PR keeps its start of the line and this PR's command. The new test here pins the start of the hook line in one snapshot line, for beforeEach. That line changes with it. For the node:test hook, the test compares only the hint.
• bun test: keep the result of a callback that throws after its timeout #47867 adds four snapshot lines with the old command to test/js/bun/test/test-test.test.ts. bun test: identify failed hooks and their files #43843 has one in test/cli/test/test-timeout-behavior.test.ts. Neither file is in this PR, so git reports no conflict. The PR that merges second must update those lines. Neither PR has a note about this.
• bun test: nest results under describe scopes (streaming POC, rebased) #48075 and bun test: buffer each file's results as a nested describe tree (buffered POC, rebased) #48076 move both literals.
• Before the merge: rebase, then git grep -n "remove the last parameter" -- src test must print nothing.

Not changed

• What a row of a .each table receives. test.each does not support tuples with optional elements at the end #24347 and bun:test: support optional trailing tuple elements in test.each #31618 own that.
• The .todo and .failing hints in the same match.
• The start of both lines. before its done callback was called is also printed if the callback calls done() after the deadline (bun test: time out a test or hook whose callback completes after its deadline #39440).
• node:test. Bun registers each node:test test and hook through a wrapper that takes done (src/js/node/test.ts:2564, :2840). If bun:test's timer ends such a test or hook, these lines are printed, even for a callback with no parameter (node:test async tests exceeding 5s timeout fail in Bun but pass in Node #27422, node:test async tests and hooks time out after 5s instead of matching Node.js no-timeout default #28190). In that case the new command has nothing to remove. For a hook before((context, done) => {}) the command is right: context gets a value.

What each callback receives

On a debug build of this branch. "After the command" is the same callback without the parameters that get no value.

Callback	Arguments	After the command
test("x", (a, b) => {})	function, undefined	() => {} passes
test.each([[1, 2]])("x", (a, b, c, d) => {})	number, number, function, undefined	(a, b) => {} passes
test.each([{ a: 1 }])("x", (row, extra) => {})	object, function	row => {} passes
beforeEach((a, b) => {})	function, undefined	() => {} passes
before((a, b) => {}) from node:test	object, function	a => {} passes

With the old command (remove the last parameter), rows 1, 2 and 4 print the same line again.

Run by hand, not in the test:

• test.each([[1, 2], [3]])("x", (a, b) => {}): the row [3] receives number, function.
• test.each([[1, 2], [1, 2, undefined]])("x", (a, b, c, d) => {}): number, number, function, undefined and number, number, undefined, function.
• test.each([[]])("x", value => {}): function.
• A tagged-template row with (row, extra): object, function.

The new test has no table with rows of different lengths. No test on main pins what such a row receives, and #24347 is open.

An option

The hint can end with , or give them a default value. A parameter with a default value is not counted, so test.each([[1, 2, 3], [4, 5]])("x", (a, b, c = undefined) => {}) passes and still reads the third column. This PR does not add it, because it is the way out for a short row, and #24347 decides what a short row receives.

Jest

Jest 30.5.2 passes done in the same positions. It prints Exceeded timeout of 300 ms for a test while waiting for `done()` to be called. with a code frame at the test.each call, and no advice about parameters. The command is Bun's own (#22534).

Test runs

• bun bd test test/js/bun/test/bun_test.test.ts with src/ of main: 6 pass, 2 fail. Only the hint text differs: 2 lines in describe/test, and each hint in the new test. With this branch: 8 pass. Release builds give the same counts.
• Debug build of this branch: jest-each.test.ts (35 pass), jest-hooks.test.ts (25 pass, 1 todo), test-failing.test.ts (8 pass), test-error-code-done-callback.test.ts (3 pass), done-async.test.ts (1 pass), test/js/node/test_runner/node-test.test.ts (45 pass).
• expect-deferred.test.ts (1 of 56 tests) and test/cli/test/bun-test.test.ts (3 of 235 tests, then 7) had 5 s timeouts on the debug build. The tests that timed out do not print this hint, and they changed from run to run. The test of expect-deferred.test.ts passes alone.

Cost

• Per call: nothing. Bun prints the line only for a test that fails on this timeout.
• Size: four strings (two lines, each with and without colors) are 14 bytes longer each. Release builds with --lto=off of 98cda0d and of 98cda0d with this change, size of bun: text 138,833,326 and 138,833,390, data 722,228 and bss 1,862,416 in both, stripped file 139,622,760 bytes in both. Debug builds: text +40 bytes. The branch was rebased after the measurement.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Timeout guidance for tests and hooks that fail before invoking a done callback now advises removing callback parameters that receive no value. Tests cover Bun callbacks, test.each, beforeEach, and node:test’s before hook, including equivalent callbacks without those parameters.

Changes

Test timeout guidance

Layer / File(s)	Summary
Timeout guidance and regression coverage src/runtime/cli/test_command.rs, test/js/bun/test/bun_test.test.ts	Timeout messages and snapshots now advise removing callback parameters that receive no value. Tests check the updated hints and verify that equivalent callbacks without those parameters pass.

Possibly related issues

• oven-sh/bun#44552 — This PR updates the timeout hints described in the issue, but does not fix the callback classification.

Suggested reviewers: jarred-sumner

Merge Risk: Minimal · up to e88a6

Merge Risk: ⚪ Minimal · up to e88a6

The timeout guidance has regression coverage for the reported case and is ready to merge after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the main change: updating the bun test done-callback timeout hint to specify which parameters to remove.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description clearly explains the problem, implementation, scope, limitations, and verification results. It does not use the exact template headings, but it provides the required information in equ…

Full details: Description check

Explanation

The description clearly explains the problem, implementation, scope, limitations, and verification results. It does not use the exact template headings, but it provides the required information in equivalent sections.

• Autofix · 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

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: robobun/afe11168/done-timeout-hint
• File: .coderabbit.yaml
• Commit: 227c193
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 27eaa493-47c6-4622-826e-7946416f6c97

Commits

Reviewing files that changed from the base of the PR and between 89452d2 and 227c193.

Files selected for processing: 2

• src/runtime/cli/test_command.rs
• test/js/bun/test/bun_test.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.

[robobun]

^{Updated 11:25 PM PT - Nov 10th, 2026}

@robobun, your commit e88a614 is building: #140062

[robobun]

Status: ready for review.

How I reproduced it on a release build of main (98cda0d), with bun test x.test.js --timeout 300:

import { test } from "bun:test";

test("x", (a, b) => {
  console.log(typeof a, typeof b);
});

function undefined
(fail) x
  ^ this test timed out after 300ms, before its done callback was called. If a done callback was not intended, remove the last parameter from the test callback function

Without b, the test prints the same line again, because done is in a. With this branch the line ends with remove the parameters that the test callback function gets no value for, and the test passes without a and b.

PR: #48536

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also swept the repo for the old "remove the last parameter" wording — no stale assertions, docs, or comments remain — and checked the --timeout 1 run: every fixture callback never calls done, so the timeout is the intended outcome rather than a speed-dependent race. I did not run the test suite; the inline snapshots were checked by reading.

Extended reasoning...

The change rewords two hint literals in src/runtime/cli/test_command.rs with no logic change, updates two existing inline snapshots, and adds one subprocess snapshot test covering test, test.each (array and object rows), beforeEach, and a node:test before hook. It touches no security-sensitive surface. The inline finding about the node:test lines in the new snapshot is the reason a human look is still worthwhile.

[claude]

Code review found no issues

No high-confidence issues detected in this change.