diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 98f2ae78561b..43e0058b7236 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -1241,7 +1241,7 @@ impl BunTest { } PromiseStatus::Fulfilled => { // Do not register a then callback when it's already fulfilled. - return Some(cfg_data); + // Fall through: a pending done callback still has to be awaited. } PromiseStatus::Rejected => { let value = bun_jsc::JSPromise::opaque_mut(promise).result(global_this.vm()); @@ -1251,6 +1251,7 @@ impl BunTest { // We previously marked it as handled above. + // Fail fast without waiting for done(), like bun_test_catch. return Some(cfg_data); } } diff --git a/test/js/bun/test/done-async.test.ts b/test/js/bun/test/done-async.test.ts index 8a46547be09c..bdfe7a329466 100644 --- a/test/js/bun/test/done-async.test.ts +++ b/test/js/bun/test/done-async.test.ts @@ -1,4 +1,4 @@ -import { expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; import path from "path"; @@ -28,3 +28,53 @@ test("done() causes the test to fail when it should", async () => { expect(result.stderr.toString()).toContain(" 7 fail\n"); expect(result.stderr.toString()).toContain(" 0 pass\n"); }); + +// A test that takes `done` and returns a promise completes when both settle. +// An already-fulfilled promise must not end the test before done() is called. +describe.each(["serial", "--concurrent"])("a returned fulfilled promise still waits for done() (%s)", mode => { + test("done() is awaited and done(err) fails its own test", async () => { + using dir = tempDir("done-and-promise", { + "done.test.ts": ` + import { test, expect } from "bun:test"; + test("never calls done", done => { + return Promise.resolve(1); + }, 50); + test("late done(err)", done => { + return Promise.resolve().then(() => { + setTimeout(() => { + try { expect(1).toBe(2); done(); } catch (e) { done(e); } + }, 10); + }); + }); + test("late done()", done => { + return Promise.resolve().then(() => { + setTimeout(() => done(), 10); + }); + }); + test("innocent", async () => { + await new Promise(resolve => setTimeout(resolve, 100)); + }); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", ...(mode === "serial" ? [] : [mode]), "done.test.ts"], + cwd: String(dir), + stdout: "ignore", + stderr: "pipe", + env: bunEnv, + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + const lines = stderr.split("\n").filter(line => /^\((pass|fail)\)/.test(line)); + // Under --concurrent a late done(err) is still reported between tests, not + // against its own test. That attribution is a separate fix. + const results = lines.map(line => line.replace(/ \[[\d.]+m?s\]$/, "")).sort(); + expect(mode === "serial" ? results : results.filter(line => !line.endsWith("late done(err)"))).toEqual( + mode === "serial" + ? ["(fail) late done(err)", "(fail) never calls done", "(pass) innocent", "(pass) late done()"] + : ["(fail) never calls done", "(pass) innocent", "(pass) late done()"], + ); + expect(stderr).toContain("timed out after 50ms, before its done callback was called"); + expect(stderr).toContain("Expected: 2"); + expect(exitCode).toBe(1); + }); +});