node:test: support the (t, done) callback signature in tests and hooks

docs/guides/util/base64.mdx

@@ -40,10 +40,11 @@ const text = bytes.toString("utf8");
 <Warning>
   The older [`btoa()`](https://developer.mozilla.org/en-US/docs/Web/API/Window/btoa) and [`atob()`](https://developer.mozilla.org/en-US/docs/Web/API/Window/atob) APIs are still available for compatibility, but they operate on binary strings instead of byte arrays. Avoid them in new code, especially when handling arbitrary binary data or non-ASCII text.
 
-  ```ts
-  const encoded = btoa("bun"); // => "YnVu"
-  const decoded = atob(encoded); // => "bun"
-  ```
+```ts
+const encoded = btoa("bun"); // => "YnVu"
+const decoded = atob(encoded); // => "bun"
+```
+
 </Warning>
 
 ---

src/js/node/net.ts

@@ -2510,7 +2510,7 @@ function lookupAndConnect(self, options) {
     if (!self.connecting) return;
     if (err) {
       process.nextTick(destroyNT, self, err);
-    } else if (!isIP(ip)) {
+    } else if (typeof ip !== "string" || !isIP(ip)) {
       err = $ERR_INVALID_IP_ADDRESS(ip);
       process.nextTick(destroyNT, self, err);
     } else if (addressType !== 4 && addressType !== 6) {

src/js/node/test.ts

@@ -659,6 +659,73 @@ function parseTestOptions(arg0: unknown, arg1: unknown, arg2: unknown) {
   return { name, options: options as TestOptions, fn };
 }
 
+// Runs a user test or hook function and completes via `finish`, a single-shot
+// finalizer (safe to call more than once). A function declaring exactly two
+// parameters (`(context, done)`) uses Node's error-first callback style: it
+// receives a `done` callback and only completes once `done` is called (a
+// truthy argument fails, a falsy one passes). Like Node, a synchronous throw
+// or a returned Promise takes precedence over a `done` call made before the
+// function returned, and a second `done` call throws. Any other arity
+// completes synchronously or when its returned Promise settles.
+function runWithDone(fn: TestFn | HookFn, context: TestContext, finish: DoneCallback) {
+  if (fn.length === 2) {
+    let returned = false;
+    let doneCalls = 0;
+    let donePending = false;
+    let doneFailure: unknown;
+    const done = (error?: unknown) => {
+      doneCalls += 1;
+      if (doneCalls > 1) {
+        if (doneCalls === 2) {
+          throw new Error("callback invoked multiple times");
+        }
+        return;
+      }
+      const failure = error ? error : undefined;
+      if (returned) {
+        finish(failure);
+      } else {
+        donePending = true;
+        doneFailure = failure;
+      }
+    };
+
+    let result: unknown;
+    try {
+      result = fn(context, done);
+    } catch (error) {
+      finish(error);
+      return;
+    }
+    returned = true;
+    if (result instanceof Promise) {
+      // Node reports this misuse right away; the promise's own outcome no
+      // longer decides the test, so don't leave its rejection unhandled.
+      (result as Promise<unknown>).then(kDefaultFunction, kDefaultFunction);
+      finish(new Error("passed a callback but also returned a Promise"));
+    } else if (donePending) {
+      finish(doneFailure);
+    }
+    return;
+  }
+
+  let result: unknown;
+  try {
+    result = fn(context);
+  } catch (error) {
+    finish(error);
+    return;
+  }
+  if (result instanceof Promise) {
+    (result as Promise<unknown>).then(
+      () => finish(),
+      error => finish(error),
+    );
+  } else {
+    finish();
+  }
+}
+
 function createTest(arg0: unknown, arg1: unknown, arg2: unknown) {
   const { name, options, fn } = parseTestOptions(arg0, arg1, arg2);
 
@@ -668,26 +735,18 @@ function createTest(arg0: unknown, arg1: unknown, arg2: unknown) {
   const runTest = (done: (error?: unknown) => void) => {
     const originalContext = ctx;
     ctx = context;
+    let finished = false;
     const endTest = (error?: unknown) => {
+      if (finished) return;
+      finished = true;
       try {
         done(error);
       } finally {
         ctx = originalContext;
       }
     };
 
-    let result: unknown;
-    try {
-      result = fn(context);
-    } catch (error) {
-      endTest(error);
-      return;
-    }
-    if (result instanceof Promise) {
-      (result as Promise<unknown>).then(() => endTest()).catch(error => endTest(error));
-    } else {
-      endTest();
-    }
+    runWithDone(fn, context, endTest);
   };
 
   return { name, options, fn: runTest };
@@ -739,25 +798,23 @@ function createHook(arg0: unknown, arg1: unknown) {
   const { fn, options } = parseHookOptions(arg0, arg1);
 
   const runHook = (done: (error?: unknown) => void) => {
-    let result: unknown;
-    try {
-      result = fn();
-    } catch (error) {
+    let finished = false;
+    const endHook = (error?: unknown) => {
+      if (finished) return;
+      finished = true;
       done(error);
-      return;
-    }
-    if (result instanceof Promise) {
-      (result as Promise<unknown>).then(() => done()).catch(error => done(error));
-    } else {
-      done();
-    }
+    };
+
+    const context = new TestContext(false, undefined, Bun.main, ctx);
+    runWithDone(fn, context, endHook);
   };
 
   return { options, fn: runHook };
 }
 
-type TestFn = (ctx: TestContext) => unknown | Promise<unknown>;
-type HookFn = () => unknown | Promise<unknown>;
+type DoneCallback = (error?: unknown) => void;
+type TestFn = (ctx: TestContext, done?: DoneCallback) => unknown | Promise<unknown>;
+type HookFn = (ctx: TestContext, done?: DoneCallback) => unknown | Promise<unknown>;
 
 type TestOptions = {
   concurrency?: number | boolean | null;

test/js/node/net/net-connect-lookup.test.ts

@@ -0,0 +1,54 @@
+import { expect, it } from "bun:test";
+import { connect } from "node:net";
+
+it("emits ERR_INVALID_IP_ADDRESS when a custom lookup yields a non-string address", async () => {
+  // Node validates `typeof ip === "string"` before isIP(); an array like
+  // ["127.0.0.1"] stringifies into a valid IP, so it must not connect.
+  for (const family of [4, 6] as const) {
+    const { promise, resolve, reject } = Promise.withResolvers<Error & { code?: string }>();
+    const socket = connect({
+      host: "example.com",
+      port: 80,
+      family,
+      lookup: (_host, _options, callback) => {
+        callback(null, ["127.0.0.1"] as unknown as string, family);
+      },
+    });
+    socket.on("connect", () => reject(new Error("connected with an invalid lookup result")));
+    socket.on("error", resolve);
+    try {
+      const error = await promise;
+      expect(error.code).toBe("ERR_INVALID_IP_ADDRESS");
+    } finally {
+      socket.destroy();
+    }
+  }
+});
+
+it("passes a string address from a custom lookup through to the connection", async () => {
+  // A loopback server observes the connection even though the hostname never
+  // resolves, proving the lookup result is what gets connected to.
+  using server = Bun.listen({
+    hostname: "127.0.0.1",
+    port: 0,
+    socket: {
+      data() {},
+    },
+  });
+  const { promise, resolve, reject } = Promise.withResolvers<void>();
+  const socket = connect({
+    host: "definitely-not-resolvable.example.invalid",
+    port: server.port,
+    family: 4,
+    lookup: (_host, _options, callback) => {
+      callback(null, "127.0.0.1", 4);
+    },
+  });
+  socket.on("connect", () => resolve());
+  socket.on("error", reject);
+  try {
+    await promise;
+  } finally {
+    socket.destroy();
+  }
+});

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

@@ -1,6 +1,6 @@
 import { spawn } from "bun";
 import { describe, expect, test } from "bun:test";
-import { bunEnv, bunExe } from "harness";
+import { bunEnv, bunExe, tempDir } from "harness";
 import { join } from "node:path";
 
 describe("node:test", () => {
@@ -226,3 +226,164 @@ test("the call record is pushed after the implementation runs, like node", () =>
   expect(f.mock.callCount()).toBe(1);
   mock.reset();
 });
+
+describe.concurrent("node:test done callback", () => {
+  async function runInlineTest(source: string) {
+    using dir = tempDir("node-test-done", { "done.test.js": source });
+    await using proc = spawn({
+      cmd: [bunExe(), "test", join(String(dir), "done.test.js")],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return { stdout, stderr, exitCode };
+  }
+
+  test("passes when done() is called synchronously, asynchronously, or with a falsy value", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      test('sync done', (t, done) => { done(); });
+      test('async done', (t, done) => { setImmediate(done); });
+      test('falsy argument passes', (t, done) => { setImmediate(() => done(0)); });
+    `);
+    // Without done support every test here throws "done is not a function".
+    expect(stderr).toContain("(pass) sync done");
+    expect(stderr).toContain("(pass) async done");
+    expect(stderr).toContain("(pass) falsy argument passes");
+    expect(stderr).toContain("0 fail");
+    expect(exitCode).toBe(0);
+  });
+
+  test("done() passes a test while done(error) or a truthy value fails others", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      test('resolves with done', (t, done) => { done(); });
+      test('rejects with an error', (t, done) => { done(new Error('boom-error')); });
+      test('rejects with a truthy value', (t, done) => { setImmediate(() => done('string-failure')); });
+    `);
+    // Without the fix 'resolves with done' throws instead of passing.
+    expect(stderr).toContain("(pass) resolves with done");
+    expect(stderr).toContain("(fail) rejects with an error");
+    expect(stderr).toContain("(fail) rejects with a truthy value");
+    expect(stderr).toContain("1 pass");
+    expect(stderr).toContain("2 fail");
+    expect(exitCode).not.toBe(0);
+  });
+
+  test("a failure reported through done(error) from a timer fails the test", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      test('async callback test that should FAIL', (t, done) => {
+        setTimeout(() => {
+          if (1 + 1 !== 3) return done(new Error('expected 3, got 2'));
+          done();
+        }, 20);
+      });
+    `);
+    // Without the fix the test passes before the timer fires and the process
+    // exits 0 with "1 pass".
+    expect(stderr).toContain("expected 3, got 2");
+    expect(stderr).toContain("1 fail");
+    expect(exitCode).not.toBe(0);
+  });
+
+  test("an exception thrown from an async callback while the test is pending fails it", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      import assert from 'node:assert';
+      test('throws before done', (t, done) => {
+        setTimeout(() => {
+          assert.ok(false, 'boom-async-throw');
+          done();
+        }, 1);
+      });
+    `);
+    // Without the fix the test completes synchronously and the process exits
+    // before the timer runs, reporting "1 pass".
+    expect(stderr).toContain("boom-async-throw");
+    expect(stderr).toContain("1 fail");
+    expect(exitCode).not.toBe(0);
+  });
+
+  test("times out when done is never called", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      test('never done', { timeout: 100 }, (t, done) => {});
+    `);
+    // Without the fix this resolves synchronously and passes (0 fail); with the
+    // fix it waits for a done that never arrives and times out.
+    expect(stderr).toContain("1 fail");
+    expect(exitCode).not.toBe(0);
+  });
+
+  test("fails when a callback-style test also returns a Promise", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      test('cb and promise', async (t, done) => { done(); });
+    `);
+    expect(stderr).toContain("passed a callback but also returned a Promise");
+    expect(stderr).toContain("1 fail");
+    expect(exitCode).not.toBe(0);
+  });
+
+  test("arity-1 tests receive the context, not done", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      test('no done', t => { if (typeof t !== 'object' || t === null) throw new Error('expected a context'); });
+    `);
+    expect(stderr).toContain("1 pass");
+    expect(stderr).toContain("0 fail");
+    expect(exitCode).toBe(0);
+  });
+
+  test("a function declaring more than two parameters is not callback style", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      import assert from 'node:assert';
+      test('arity 3', (t, done, extra) => {
+        assert.strictEqual(done, undefined);
+        assert.strictEqual(extra, undefined);
+      });
+    `);
+    // Node only enables callback mode for exactly two parameters.
+    expect(stderr).toContain("1 pass");
+    expect(stderr).toContain("0 fail");
+    expect(exitCode).toBe(0);
+  });
+
+  test("calling done() a second time throws like Node", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import test from 'node:test';
+      import assert from 'node:assert';
+      test('second done throws', (t, done) => {
+        done();
+        assert.throws(() => done(), /callback invoked multiple times/);
+      });
+    `);
+    expect(stderr).toContain("1 pass");
+    expect(stderr).toContain("0 fail");
+    expect(exitCode).toBe(0);
+  });
+
+  test("hooks receive a context object and a done callback", async () => {
+    const { stderr, exitCode } = await runInlineTest(`
+      import { test, before, beforeEach } from 'node:test';
+      import assert from 'node:assert';
+      const order = [];
+      before((ctx, done) => {
+        if (typeof ctx !== 'object' || ctx === null) return done(new Error('expected a hook context'));
+        setImmediate(() => { order.push('before'); done(); });
+      });
+      beforeEach((ctx, done) => { setImmediate(() => { order.push('beforeEach'); done(); }); });
+      test('runs after the hooks completed', () => {
+        assert.deepStrictEqual(order, ['before', 'beforeEach']);
+      });
+    `);
+    // Without the fix the hooks complete before their setImmediate callbacks
+    // run, so the test observes an empty order array and fails.
+    expect(stderr).toContain("(pass) runs after the hooks completed");
+    expect(stderr).toContain("0 fail");
+    expect(exitCode).toBe(0);
+  });
+});