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,26 @@ 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 {
+      const promise = result as Promise<unknown>;
+      if (useCallback) {
+        // Callback-style tests complete only when done() is invoked; the
+        // returned promise must not auto-complete the test on resolution.
+        // A rejection is still surfaced as a failure.
+        promise.catch(error => endTest(error));
+      } else {
+        promise.then(() => endTest(), error => endTest(error));
+      }
+    } 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 +778,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,61 @@
+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: the test function returns a promise that
+// resolves BEFORE done() is called. This proves the runner waits for done()
+// and does not auto-complete on promise resolution: `sawDone` is only true
+// once the deferred done() actually runs, and it is asserted on the next
+// microtask-draining tick via a second callback test below.
+let sawDone = false;
+test("callback test waits for done() even when the body returns a promise", (t, done) => {
+  // Returning a resolved promise must NOT complete the test; only done() may.
+  return Promise.resolve().then(() => {
+    setTimeout(() => {
+      callCount++;
+      sawDone = true;
+      done();
+    }, 5);
+  });
+});
+
+// Runs after the previous test. If the runner had wrongly completed the
+// prior test on promise-resolution (before done()), `sawDone` would still be
+// false here, failing the assertion.
+test("previous callback test only completed via done()", (t, done) => {
+  assert.equal(sawDone, true, "async callback test completed before done() was called");
+  callCount++;
+  done();
+});
+
+// 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++;
+});
+
+// Calling done() twice (or done() plus a thrown/rejected value) must not
+// double-complete or double-count the test.
+test("done() is idempotent", (t, done) => {
+  callCount++;
+  done();
+  done();
+});
+
+process.on("exit", () => {
+  assert.equal(callCount, 6, `expected 6 test invocations, saw ${callCount}`);
+});

test/js/node/test_runner/fixtures/07-callback-done-error.js

@@ -0,0 +1,6 @@
+const { test } = require("node:test");
+
+// Callback-style test that reports failure via done(error) must fail.
+test("callback test fails when done() is called with an error", (t, done) => {
+  done(new Error("boom"));
+});

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

@@ -52,6 +52,21 @@ 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"),
+    });
+  });
+
+  test("should report failure when a callback-style test calls done(error)", async () => {
+    const { exitCode, stderr } = await runTests(["07-callback-done-error.js"]);
+    expect(exitCode).not.toBe(0);
+    expect(stderr).toContain("1 fail");
+    expect(stderr).toContain("boom");
+  });
 });
 
 async function runTests(filenames: string[]) {