fix(bun/test): it.failing for tests using done callbacks (#19018)

packages/bun-types/test.d.ts

@@ -426,8 +426,13 @@ declare module "bun:test" {
      *
      * @param label the label for the test
      * @param fn the test function
+     * @param options the test timeout or options
      */
-    failing(label: string, fn?: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void)): void;
+    failing(
+      label: string,
+      fn?: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
+      options?: number | TestOptions,
+    ): void;
     /**
      * Runs this test, if `condition` is true.
      *

src/bun.js/bindings/JSGlobalObject.zig

@@ -460,6 +460,18 @@ pub const JSGlobalObject = opaque {
         return JSGlobalObject__clearException(this);
     }
 
+    /// Clear the currently active exception off the VM unless it is a
+    /// termination exception.
+    ///
+    /// Returns `true` if the exception was cleared, `false` if it was a
+    /// termination exception. Use `clearException` to unconditionally clear
+    /// exceptions.
+    ///
+    /// It is safe to call this function when no exception is present.
+    pub fn clearExceptionExceptTermination(this: *JSGlobalObject) bool {
+        return JSGlobalObject__clearExceptionExceptTermination(this);
+    }
+
     /// Clears the current exception and returns that value. Requires compile-time
     /// proof of an exception via `error.JSError`
     pub fn takeException(this: *JSGlobalObject, proof: bun.JSError) JSValue {
@@ -727,6 +739,7 @@ pub const JSGlobalObject = opaque {
     extern fn JSC__JSGlobalObject__vm(*JSGlobalObject) *VM;
     extern fn JSC__JSGlobalObject__deleteModuleRegistryEntry(*JSGlobalObject, *const ZigString) void;
     extern fn JSGlobalObject__clearException(*JSGlobalObject) void;
+    extern fn JSGlobalObject__clearExceptionExceptTermination(*JSGlobalObject) bool;
     extern fn JSGlobalObject__clearTerminationException(this: *JSGlobalObject) void;
     extern fn JSGlobalObject__hasException(*JSGlobalObject) bool;
     extern fn JSGlobalObject__setTimeZone(this: *JSGlobalObject, timeZone: *const ZigString) bool;

src/bun.js/bindings/bindings.cpp

@@ -6272,6 +6272,11 @@ extern "C" void JSGlobalObject__clearException(JSC::JSGlobalObject* globalObject
     DECLARE_CATCH_SCOPE(globalObject->vm()).clearException();
 }
 
+extern "C" bool JSGlobalObject__clearExceptionExceptTermination(JSC::JSGlobalObject* globalObject)
+{
+    return DECLARE_CATCH_SCOPE(globalObject->vm()).clearExceptionExceptTermination();
+}
+
 extern "C" JSC::EncodedJSValue JSGlobalObject__tryTakeException(JSC::JSGlobalObject* globalObject)
 {
     auto scope = DECLARE_CATCH_SCOPE(globalObject->vm());

src/bun.js/test/jest.zig

@@ -259,6 +259,7 @@ pub const TestRunner = struct {
         status: Status = Status.pending,
 
         pub const ID = u32;
+        pub const null_id: ID = std.math.maxInt(Test.ID);
         pub const List = std.MultiArrayList(Test);
 
         pub const Status = enum(u4) {
@@ -633,21 +634,35 @@ pub const TestScope = struct {
 
         if (JSC.getFunctionData(function)) |data| {
             var task = bun.cast(*TestRunnerTask, data);
+            const expect_count = expect.active_test_expectation_counter.actual;
+            const current_test = task.testScope();
+            const no_err_result: Result = if (current_test.tag == .fail)
+                .{ .fail_because_failing_test_passed = expect_count }
+            else
+                .{ .pass = expect_count };
 
             JSC.setFunctionData(function, null);
             if (args.len > 0) {
                 const err = args.ptr[0];
                 if (err.isEmptyOrUndefinedOrNull()) {
                     debug("done()", .{});
-                    task.handleResult(.{ .pass = expect.active_test_expectation_counter.actual }, .callback);
+                    task.handleResult(no_err_result, .callback);
                 } else {
                     debug("done(err)", .{});
-                    _ = globalThis.bunVM().uncaughtException(globalThis, err, true);
-                    task.handleResult(.{ .fail = expect.active_test_expectation_counter.actual }, .callback);
+                    const result: Result = if (current_test.tag == .fail) failing_passed: {
+                        break :failing_passed if (globalThis.clearExceptionExceptTermination())
+                            Result{ .pass = expect_count }
+                        else
+                            Result{ .fail = expect_count }; // what is the correct thing to do when terminating?
+                    } else passing_failed: {
+                        _ = globalThis.bunVM().uncaughtException(globalThis, err, true);
+                        break :passing_failed Result{ .fail = expect_count };
+                    };
+                    task.handleResult(result, .callback);
                 }
             } else {
                 debug("done()", .{});
-                task.handleResult(.{ .pass = expect.active_test_expectation_counter.actual }, .callback);
+                task.handleResult(no_err_result, .callback);
             }
         }
 
@@ -1142,7 +1157,7 @@ pub const DescribeScope = struct {
             if (end == 0) {
                 var runner = allocator.create(TestRunnerTask) catch unreachable;
                 runner.* = .{
-                    .test_id = std.math.maxInt(TestRunner.Test.ID),
+                    .test_id = TestRunner.Test.null_id,
                     .describe = this,
                     .globalThis = globalObject,
                     .source_file_path = source.path.text,
@@ -1174,8 +1189,8 @@ pub const DescribeScope = struct {
 
     pub fn onTestComplete(this: *DescribeScope, globalThis: *JSGlobalObject, test_id: TestRunner.Test.ID, skipped: bool) void {
         // invalidate it
-        this.current_test_id = std.math.maxInt(TestRunner.Test.ID);
-        if (test_id != std.math.maxInt(TestRunner.Test.ID)) this.pending_tests.unset(test_id);
+        this.current_test_id = TestRunner.Test.null_id;
+        if (test_id != TestRunner.Test.null_id) this.pending_tests.unset(test_id);
         globalThis.bunVM().onUnhandledRejectionCtx = null;
 
         if (!skipped) {
@@ -1277,6 +1292,10 @@ pub const TestRunnerTask = struct {
         fulfilled,
     };
 
+    pub inline fn testScope(this: *TestRunnerTask) *TestScope {
+        return &this.describe.tests.items[this.test_id];
+    }
+
     pub fn onUnhandledRejection(jsc_vm: *VirtualMachine, globalObject: *JSGlobalObject, rejection: JSValue) void {
         var deduped = false;
         const is_unhandled = jsc_vm.onUnhandledRejectionCtx == null;
@@ -1309,7 +1328,11 @@ pub const TestRunnerTask = struct {
         if (jsc_vm.onUnhandledRejectionCtx) |ctx| {
             var this = bun.cast(*TestRunnerTask, ctx);
             jsc_vm.onUnhandledRejectionCtx = null;
-            this.handleResult(.{ .fail = expect.active_test_expectation_counter.actual }, .unhandledRejection);
+            const result: Result = if (this.testScope().tag == .fail)
+                .{ .pass = expect.active_test_expectation_counter.actual }
+            else
+                .{ .fail = expect.active_test_expectation_counter.actual };
+            this.handleResult(result, .unhandledRejection);
         } else if (Jest.runner) |runner| {
             if (!deduped)
                 runner.unhandled_errors_between_tests += 1;
@@ -1343,7 +1366,7 @@ pub const TestRunnerTask = struct {
         jsc_vm.last_reported_error_for_dedupe = .zero;
 
         const test_id = this.test_id;
-        if (test_id == std.math.maxInt(TestRunner.Test.ID)) {
+        if (test_id == TestRunner.Test.null_id) {
             describe.onTestComplete(globalThis, test_id, true);
             Jest.runner.?.runNextTest();
             this.deinit();

test/js/bun/test/fixtures/failing-test-done-test-fails.fixture.ts

@@ -0,0 +1,34 @@
+import { describe, test, expect } from "bun:test";
+
+describe("test.failing with a done callback", () => {
+  test.failing("fails when done is called with no args", done => {
+    done();
+  });
+
+  test.failing("fails when done is called with undefined", done => {
+    done(undefined);
+  });
+
+  test.failing("fails when all expectations are met and done is called without an error", done => {
+    expect(1).toBe(1);
+    done();
+  });
+
+  describe("when test fn is async", () => {
+    // NOTE: tests that resolve/reject immediately hit a different code path
+    test.failing("fails when done() is called immediately", async done => {
+      done();
+    });
+
+    test.failing("fails when done() is called on the next tick", async done => {
+      await new Promise(resolve => process.nextTick(resolve));
+      done();
+    });
+
+    test.failing("fails when all expectations are met and done is called", async done => {
+      await Bun.sleep(5);
+      expect(1).toBe(1);
+      done();
+    });
+  });
+});

test/js/bun/test/fixtures/failing-test-done-test-succeeds.fixture.ts

@@ -0,0 +1,42 @@
+import { describe, test } from "bun:test";
+
+describe("test.failing with a done callback", () => {
+  test.failing("passes when an error is thrown", done => {
+    throw new Error("test error");
+  });
+
+  test.failing("passes when done() is called with an error", done => {
+    done(new Error("test error"));
+  });
+
+  describe("when test fn is asynchronous but does not return a promise", () => {
+    test.failing("passes when done(err) is called on next tick", done => {
+      process.nextTick(() => {
+        done(new Error("test error"));
+      });
+    });
+
+    test.failing("passes when done(err) is called on next event loop cycle", done => {
+      setTimeout(() => {
+        done(new Error("test error"));
+      }, 0);
+    });
+  });
+
+  describe("when test fn is async", () => {
+    // NOTE: tests that resolve/reject immediately hit a different code path
+    test.failing("passes when a promise rejects", async _done => {
+      await Bun.sleep(5);
+      throw new Error("test error");
+    });
+
+    test.failing("passes when a promise rejects immediately", async _done => {
+      throw new Error("test error");
+    });
+
+    test.failing("passes when done() is called with an error", async done => {
+      await Bun.sleep(5);
+      done(new Error("test error"));
+    });
+  });
+});

test/js/bun/test/fixtures/failing-test-timeout.fixture.ts

@@ -1,4 +1,20 @@
+import { isCI, isWindows } from "harness";
+
 jest.setTimeout(5);
-test.failing("timeouts still count as failures", async () => {
-  await Bun.sleep(1000);
+
+describe("test.failing", () => {
+  test.failing("Timeouts still count as failures", async () => {
+    await Bun.sleep(1000);
+  });
+
+  // fixme: hangs on windows. Timer callback never fires
+  describe.skipIf(isWindows && isCI)("when using a done() callback", () => {
+    test.failing("fails when an async test never calls done()", async _done => {
+      // nada
+    });
+
+    test.failing("fails when a sync test never calls done()", _done => {
+      // nada
+    });
+  });
 });

test/js/bun/test/test-failing.test.ts

@@ -39,7 +39,37 @@ describe("test.failing", () => {
     if (result.exitCode === 0) {
       fail("Expected exit code to be non-zero\n\n" + stderr);
     }
-    expect(stderr).toContain(" 1 fail\n");
+    expect(stderr).toContain(" 0 pass\n");
     expect(stderr).toMatch(/timed out after \d+ms/i);
   });
+
+  describe("when using a done() callback", () => {
+    it("when a test throws, rejects, or passes an error to done(), the test passes", async () => {
+      const result = await $.cwd(
+        fixtureDir,
+      ).nothrow()`${bunExe()} test ./failing-test-done-test-succeeds.fixture.ts`.quiet();
+      const stderr = result.stderr.toString();
+      try {
+        expect(stderr).toContain("0 fail");
+        expect(result.exitCode).toBe(0);
+      } catch (e) {
+        console.error(stderr);
+        throw e;
+      }
+    });
+
+    it("when the test doesn't throw, or otherwise fail, the test does not pass", async () => {
+      const result = await $.cwd(
+        fixtureDir,
+      ).nothrow()`${bunExe()} test ./failing-test-done-test-fails.fixture.ts`.quiet();
+      const stderr = result.stderr.toString();
+      try {
+        expect(stderr).toContain("0 pass");
+        expect(result.exitCode).not.toBe(0);
+      } catch (e) {
+        console.error(stderr);
+        throw e;
+      }
+    });
+  });
 });

test/js/third_party/express/express.text.test.ts

@@ -509,7 +509,7 @@ describe("express.text()", function () {
   });
 });
 
-function createApp(options) {
+function createApp(options?) {
   var app = express();
 
   app.use(express.text(options));