Merge branch 'main' into don/test/done-cb

src/bun.js/test/jest.zig

@@ -648,15 +648,11 @@ pub const TestScope = struct {
                     task.handleResult(no_err_result, .callback);
                 } else {
                     debug("done(err)", .{});
-                    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 };
-                    };
+                    _ = globalThis.bunVM().uncaughtException(globalThis, err, true);
+                    const result: Result = if (current_test.tag == .fail)
+                        Result{ .pass = expect_count }
+                    else
+                        Result{ .fail = expect_count };
                     task.handleResult(result, .callback);
                 }
             } else {

test/js/bun/test/done-async.test.ts

@@ -1,30 +0,0 @@
-import { expect, test } from "bun:test";
-
-import { bunEnv, bunExe, tempDirWithFiles } from "harness";
-import path from "path";
-
-test("done() causes the test to fail when it should", async () => {
-  const dir = tempDirWithFiles("done", {
-    "done.test.ts": await Bun.file(path.join(import.meta.dir, "done-infinity.fixture.ts")).text(),
-    "package.json": JSON.stringify({
-      name: "done",
-      version: "0.0.0",
-      scripts: {
-        test: "bun test",
-      },
-    }),
-  });
-
-  const $$ = new Bun.$.Shell();
-  $$.nothrow();
-  $$.cwd(dir);
-  $$.env(bunEnv);
-  const result = await $$`${bunExe()} test`;
-
-  console.log(result.stdout.toString());
-  console.log(result.stderr.toString());
-
-  expect(result.exitCode).toBe(1);
-  expect(result.stderr.toString()).toContain(" 7 fail\n");
-  expect(result.stderr.toString()).toContain(" 0 pass\n");
-});

test/js/bun/test/done-callback.test.ts

@@ -1,18 +1,125 @@
-import { test, expect } from "bun:test";
 import { bunEnv, bunExe } from "harness";
-import path from "path";
+import { join } from "path";
 
-test("verify we print error messages passed to done callbacks", () => {
-  const { stdout, stderr } = Bun.spawnSync({
-    cmd: [bunExe(), "test", path.resolve(import.meta.dir, "test-error-done-callback-fixture.ts")],
-    env: { ...bunEnv, BUN_JSC_showPrivateScriptsInStackTraces: "0" },
-    stdout: "pipe",
-    stderr: "pipe",
+const fixtureDir = join(import.meta.dir, "fixtures", "done-cb");
+var $$: typeof Bun.$;
+const bunTest = (file: string) => $$`${bunExe()} test ${file}`.quiet();
+
+beforeAll(() => {
+  $$ = new Bun.$.Shell();
+  $$.cwd(fixtureDir);
+  $$.nothrow();
+  $$.env({
+    ...bunEnv,
+    BUN_JSC_showPrivateScriptsInStackTraces: "0",
+  } as unknown as Record<string, string | undefined>);
+});
+
+describe("basic done() usage", () => {
+  describe("test will pass", () => {
+    it("when done() is called with no args", done => {
+      done();
+    });
+
+    for (const arg of [null, undefined]) {
+      it(`when done() is called with ${arg}`, done => {
+        done(arg);
+      });
+    }
+
+    // NOTE: immediately-resolving promises hit a different codepath
+    it("when a promise resolves then calls done()", done => {
+      return Bun.sleep(5).then(done);
+    });
+
+    it("when a promise resolves immediately then calls done()", done => {
+      return Promise.resolve().then(done);
+    });
+
+    it("when a promise resolves on next tick then calls done()", done => {
+      return new Promise(resolve => {
+        process.nextTick(() => resolve(done()));
+      });
+    });
+
+    it("when done() is called on next tick()", done => {
+      process.nextTick(done);
+    });
+  }); // </ test will pass>
+
+  describe("test will fail", () => {
+    it("done(err) fails the test", async () => {
+      const result = await bunTest(`./done-should-fail.fixture.ts`);
+      const stderr = result.stderr.toString();
+      const stdout = result.stdout.toString();
+      try {
+        expect(stderr).toMatch(/ \d fail\n/);
+        expect(stderr).toContain(" 0 pass\n");
+        for (let i = 0; i < 5; i++) {
+          expect(stderr).toContain(`error message ${i + 1}`);
+        }
+        expect(result.exitCode).toBe(1);
+      } catch (e) {
+        console.log(stdout);
+        console.log(stderr);
+        throw e;
+      }
+    });
+  }); // </ test will fail>
+}); // </ basic done() usage>
+
+describe("done callbacks in sync tests", () => {
+  it("test will not hang when done() is never called or called after timeout", async () => {
+    const result = await bunTest("./done-timeout-sync.fixture.ts");
+    const stderr = result.stderr.toString();
+    const stdout = result.stdout.toString();
+
+    try {
+      expect(result.exitCode).toBe(1);
+      expect(stderr).toContain(" 0 pass\n");
+      expect(stderr).toContain("timed out after");
+    } catch (e) {
+      console.log(stdout);
+      console.log(stderr);
+      throw e;
+    }
   });
+}); // </ done callbacks in sync tests>
+
+describe("done callbacks in async tests", () => {
+  it("done() causes the test to fail when it should", async () => {
+    const result = await bunTest("./done-infinity.fixture.ts");
+    const stderr = result.stderr.toString();
+    const stdout = result.stdout.toString();
+
+    try {
+      expect(stderr).toContain(" 7 fail\n");
+      expect(stderr).toContain(" 0 pass\n");
+    } catch (e) {
+      console.log(stdout);
+      console.log(stderr);
+      throw e;
+    }
+  });
+
+  it("calling done() then rejecting makes the test pass but makes `bun test` exit with 1", async () => {
+    const result = await bunTest("./done-then-reject.fixture.ts");
+    const stderr = result.stderr.toString();
+    expect(result.exitCode).toBe(1);
+    expect(stderr).toContain(" 1 pass\n");
+    expect(stderr).toContain(" 0 fail\n");
+    expect(stderr).toContain("error message from test");
+    expect(stderr).toContain("Unhandled error between tests");
+  });
+}); // </ done callbacks in async tests>
+
+test("verify we print error messages passed to done callbacks", async () => {
+  const fixtureName = "test-error-done-callback-fixture.ts";
+  const { stdout, stderr } = await bunTest(`./${fixtureName}`);
   let stdoutStr = stdout
     .toString()
     .replaceAll("\\", "/")
-    .replaceAll(import.meta.dir.replaceAll("\\", "/"), "<dir>")
+    .replaceAll(fixtureDir.replaceAll("\\", "/"), "<dir>")
     .replace(/\d+(\.\d+)?ms/g, "<time>ms")
     .replace(/\d+(\.\d+)?s/g, "<time>s")
     .replaceAll(Bun.version_with_sha, "<version>")
@@ -24,7 +131,7 @@ test("verify we print error messages passed to done callbacks", () => {
   let stderrStr = stderr
     .toString()
     .replaceAll("\\", "/")
-    .replaceAll(import.meta.dir.replaceAll("\\", "/"), "<dir>")
+    .replaceAll(fixtureDir.replaceAll("\\", "/"), "<dir>")
     .replace(/\d+(\.\d+)?ms/g, "<time>ms")
     .replace(/\d+(\.\d+)?s/g, "<time>s")
     .replaceAll(Bun.version_with_sha, "<version>")
@@ -40,7 +147,7 @@ test("verify we print error messages passed to done callbacks", () => {
   `);
   expect(stderrStr).toMatchInlineSnapshot(`
     "
-    test/js/bun/test/test-error-done-callback-fixture.ts:
+    ${fixtureName}:
     22 |   105,
     23 |   115,
     24 | );
@@ -49,7 +156,7 @@ test("verify we print error messages passed to done callbacks", () => {
     27 |   done(new Error(msg + "(sync)"));
     ^
     error: you should see this(sync)
-    at <anonymous> (<dir>/test-error-done-callback-fixture.ts:27:8)
+    at <anonymous> (<dir>/${fixtureName}:27:8)
     (fail) error done callback (sync)
     27 |   done(new Error(msg + "(sync)"));
     28 | });
@@ -59,7 +166,7 @@ test("verify we print error messages passed to done callbacks", () => {
     32 |   done(new Error(msg + "(async with await)"));
     ^
     error: you should see this(async with await)
-    at <anonymous> (<dir>/test-error-done-callback-fixture.ts:32:8)
+    at <anonymous> (<dir>/${fixtureName}:32:8)
     (fail) error done callback (async with await)
     32 |   done(new Error(msg + "(async with await)"));
     33 | });

test/js/bun/test/fixtures/done-cb/done-should-fail.fixture.ts

@@ -0,0 +1,26 @@
+let i = 0;
+const msg = () => `error message ${++i}`;
+
+describe("sync test functions", () => {
+  test(`done('some string') fails the test`, done => {
+    done(msg());
+  });
+
+  test(`done(new Error("message")) fails the test`, done => {
+    done(new Error(msg()));
+  });
+
+  test(`throwing an error fails the test`, _done => {
+    throw new Error(msg());
+  });
+});
+
+describe("async test functions", () => {
+  test("rejecting a promise fails the test", _done => {
+    return Promise.reject(new Error(msg()));
+  });
+
+  test("resolving then calling done() with an error fails the test", done => {
+    return Promise.resolve().then(() => done(new Error(msg())));
+  });
+});

test/js/bun/test/fixtures/done-cb/done-then-reject.fixture.ts

@@ -0,0 +1,4 @@
+test("calling done then rejecting", done => {
+  done();
+  return Promise.reject(new Error("error message from test"));
+});

test/js/bun/test/fixtures/done-cb/done-timeout-sync.fixture.ts

@@ -0,0 +1,9 @@
+jest.setTimeout(5);
+
+it("fails when cb is never called", done => {
+  // nada
+});
+
+it("fails when cb is called after timeout", done => {
+  setTimeout(done, 1000);
+});