diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 6a75a9768313..4e34af95336d 100644 --- a/src/js/node/test.ts +++ b/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).then(() => endTest()).catch(error => endTest(error)); - } else { + const promise = result as Promise; + 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; +type TestFn = (ctx: TestContext, done?: (error?: unknown) => void) => unknown | Promise; type HookFn = () => unknown | Promise; type TestOptions = { diff --git a/test/js/node/test_runner/fixtures/06-callback-tests.js b/test/js/node/test_runner/fixtures/06-callback-tests.js new file mode 100644 index 000000000000..f837326eeaab --- /dev/null +++ b/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}`); +}); diff --git a/test/js/node/test_runner/fixtures/07-callback-done-error.js b/test/js/node/test_runner/fixtures/07-callback-done-error.js new file mode 100644 index 000000000000..2cefa21b96be --- /dev/null +++ b/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")); +}); diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 3e422ae5ae0d..cf19003e7d48 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/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[]) {