node:test: pass a done() callback to (t, done) => {} style tests

src/js/node/test.ts

@@ -665,10 +665,22 @@ function createTest(arg0: unknown, arg1: unknown, arg2: unknown) {
   checkNotInsideTest(ctx, "test");
   const context = new TestContext(true, name, Bun.main, ctx);
 
+  // Node's node:test passes a second `done` callback argument when the test
+  // function declares two or more parameters, e.g. `test('name', (t, done) => ...)`.
+  // Detect that here so we can both forward `done` to the user and avoid
+  // auto-completing the test synchronously while it waits on the callback.
+  // https://nodejs.org/api/test.html#testname-options-fn
+  const useCallback = typeof fn === "function" && fn.length >= 2;
+
   const runTest = (done: (error?: unknown) => void) => {
     const originalContext = ctx;
     ctx = context;
+    let ended = false;
     const endTest = (error?: unknown) => {
+      // Calling done() and also returning a value (or throwing) must not
+      // complete the test twice. Make endTest idempotent.
+      if (ended) return;
+      ended = true;
       try {
         done(error);
       } finally {
@@ -678,16 +690,18 @@ function createTest(arg0: unknown, arg1: unknown, arg2: unknown) {
 
     let result: unknown;
     try {
-      result = fn(context);
+      result = useCallback ? fn(context, endTest) : fn(context);
     } catch (error) {
       endTest(error);
       return;
     }
     if (result instanceof Promise) {
       (result as Promise<unknown>).then(() => endTest()).catch(error => endTest(error));
-    } else {
+    } else if (!useCallback) {
       endTest();
     }
+    // useCallback + sync return: wait for the user to invoke done().
+    // bun:test enforces the test timeout if done is never called.
   };
 
   return { name, options, fn: runTest };
@@ -756,7 +770,7 @@ function createHook(arg0: unknown, arg1: unknown) {
   return { options, fn: runHook };
 }
 
-type TestFn = (ctx: TestContext) => unknown | Promise<unknown>;
+type TestFn = (ctx: TestContext, done?: (error?: unknown) => void) => unknown | Promise<unknown>;
 type HookFn = () => unknown | Promise<unknown>;
 
 type TestOptions = {

test/js/node/test_runner/fixtures/06-callback-tests.js

@@ -0,0 +1,39 @@
+const { test } = require("node:test");
+const assert = require("node:assert");
+
+let callCount = 0;
+
+// Sync callback-style test: invokes done() to signal completion.
+test("callback test resolves with done()", (t, done) => {
+  assert.equal(typeof done, "function");
+  callCount++;
+  done();
+});
+
+// Async-but-callback-style test: schedules done() in a setTimeout.
+test("callback test resolves with done() asynchronously", (t, done) => {
+  setTimeout(() => {
+    callCount++;
+    done();
+  }, 5);
+});
+
+// Callback-style test that signals failure via done(err) must still report
+// failure, but we cannot easily inspect that from inside the same suite.
+// The shape is exercised by the next two tests instead.
+
+// Existing (t) => {...} sync-style signature must keep working.
+test("sync test without done() still passes", t => {
+  assert.equal(typeof t.name, "string");
+  callCount++;
+});
+
+// Existing async-function signature must keep working.
+test("async test without done() still passes", async t => {
+  await Promise.resolve();
+  callCount++;
+});
+
+process.on("exit", () => {
+  assert.equal(callCount, 4, `expected 4 test invocations, saw ${callCount}`);
+});

test/js/node/test_runner/node-test.test.ts

@@ -52,6 +52,14 @@ describe("node:test", () => {
       stderr: expect.stringContaining("0 fail"),
     });
   });
+
+  test("should pass a done() callback to tests whose function takes two parameters", async () => {
+    const { exitCode, stderr } = await runTests(["06-callback-tests.js"]);
+    expect({ exitCode, stderr }).toMatchObject({
+      exitCode: 0,
+      stderr: expect.stringContaining("0 fail"),
+    });
+  });
 });
 
 async function runTests(filenames: string[]) {