From 5d71906c52d41fdf2376feb08eb401bbb92ec349 Mon Sep 17 00:00:00 2001 From: pfg Date: Tue, 23 Sep 2025 18:28:56 -0700 Subject: [PATCH 1/7] concurrent immediate fix --- src/bun.js/test/Collection.zig | 11 ++-- src/bun.js/test/Execution.zig | 6 +- src/bun.js/test/bun_test.zig | 11 ++-- .../bun/test/concurrent_immediate.fixture.ts | 15 +++++ test/js/bun/test/concurrent_immediate.test.ts | 59 +++++++++++++++++++ .../concurrent_immediate_error.fixture.ts | 15 +++++ 6 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 test/js/bun/test/concurrent_immediate.fixture.ts create mode 100644 test/js/bun/test/concurrent_immediate.test.ts create mode 100644 test/js/bun/test/concurrent_immediate_error.fixture.ts diff --git a/src/bun.js/test/Collection.zig b/src/bun.js/test/Collection.zig index a59f6b87e40..10e5422b2a1 100644 --- a/src/bun.js/test/Collection.zig +++ b/src/bun.js/test/Collection.zig @@ -137,11 +137,12 @@ pub fn step(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject this.active_scope = new_scope; group.log("collection:runOne set scope to {s}", .{this.active_scope.base.name orelse "undefined"}); - BunTest.runTestCallback(buntest_strong, globalThis, callback.get(), false, .{ - .collection = .{ - .active_scope = previous_scope, - }, - }, .epoch); + if (BunTest.runTestCallback(buntest_strong, globalThis, callback.get(), false, .{ + .collection = .{ .active_scope = previous_scope }, + }, .epoch)) |cfg_data| { + // the result is available immediately; queue + buntest.addResult(cfg_data); + } return .{ .waiting = .{} }; } diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index 3819fad8dc9..eb0b99a7e20 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -374,7 +374,11 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba }; groupLog.log("runSequence queued callback: {}", .{callback_data}); - BunTest.runTestCallback(buntest_strong, globalThis, cb.get(), next_item.has_done_parameter, callback_data, next_item.timespec); + if (BunTest.runTestCallback(buntest_strong, globalThis, cb.get(), next_item.has_done_parameter, callback_data, next_item.timespec) != null) { + // the result is available immediately; advance the sequence and run again. + this.advanceSequence(sequence, group); + return null; // run again + } return .{ .execute = .{ .timeout = next_item.timespec } }; } else { switch (next_item.base.mode) { diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index 1688ab0cd86..71f5bb6bd46 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -543,8 +543,8 @@ pub const BunTest = struct { } } - /// if sync, the result is queued and appended later - pub fn runTestCallback(this_strong: BunTestPtr, globalThis: *jsc.JSGlobalObject, cfg_callback: jsc.JSValue, cfg_done_parameter: bool, cfg_data: BunTest.RefDataValue, timeout: bun.timespec) void { + /// if sync, the result is returned. if async, null is returned. + pub fn runTestCallback(this_strong: BunTestPtr, globalThis: *jsc.JSGlobalObject, cfg_callback: jsc.JSValue, cfg_done_parameter: bool, cfg_data: BunTest.RefDataValue, timeout: bun.timespec) ?RefDataValue { group.begin(@src()); defer group.end(); const this = this_strong.get(); @@ -586,20 +586,19 @@ pub const BunTest = struct { const this_ref: *RefData = if (dcb_ref) |dcb_ref_value| dcb_ref_value.dupe() else ref(this_strong, cfg_data); result.?.then(globalThis, this_ref, bunTestThen, bunTestCatch); drain(globalThis); - return; + return null; } if (dcb_ref) |_| { // completed asynchronously group.log("callTestCallback -> wait for done callback", .{}); drain(globalThis); - return; + return null; } group.log("callTestCallback -> sync", .{}); drain(globalThis); - this.addResult(cfg_data); - return; + return cfg_data; } /// called from the uncaught exception handler, or if a test callback rejects or throws an error diff --git a/test/js/bun/test/concurrent_immediate.fixture.ts b/test/js/bun/test/concurrent_immediate.fixture.ts new file mode 100644 index 00000000000..0c9dc496a6e --- /dev/null +++ b/test/js/bun/test/concurrent_immediate.fixture.ts @@ -0,0 +1,15 @@ +beforeEach(() => { + console.log("beforeEach"); +}); +afterEach(() => { + console.log("afterEach"); +}); +test.concurrent("test 1", () => { + console.log("start test 1"); +}); +test.concurrent("test 2", () => { + console.log("start test 2"); +}); +test.concurrent("test 3", () => { + console.log("start test 3"); +}); diff --git a/test/js/bun/test/concurrent_immediate.test.ts b/test/js/bun/test/concurrent_immediate.test.ts new file mode 100644 index 00000000000..c9bf7666dde --- /dev/null +++ b/test/js/bun/test/concurrent_immediate.test.ts @@ -0,0 +1,59 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, normalizeBunSnapshot } from "harness"; + +test("describe/test", async () => { + const result = await Bun.spawn({ + cmd: [bunExe(), "test", import.meta.dir + "/concurrent_immediate.fixture.ts"], + stdout: "pipe", + stderr: "pipe", + env: bunEnv, + }); + const exitCode = await result.exited; + const stdout = await result.stdout.text(); + const stderr = await result.stderr.text(); + expect(exitCode).toBe(0); + expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(` + "bun test () + beforeEach + start test 1 + afterEach + beforeEach + start test 2 + afterEach + beforeEach + start test 3 + afterEach" + `); +}); + +test("describe/test", async () => { + const result = await Bun.spawn({ + cmd: [bunExe(), "test", import.meta.dir + "/concurrent_immediate_error.fixture.ts"], + stdout: "pipe", + stderr: "pipe", + env: bunEnv, + }); + const exitCode = await result.exited; + const stdout = await result.stdout.text(); + const stderr = await result.stderr.text(); + expect(exitCode).toBe(1); + expect(normalizeBunSnapshot(stderr)).toMatchInlineSnapshot(` + "test/js/bun/test/concurrent_immediate_error.fixture.ts: + (pass) test 1 + 6 | }); + 7 | test.concurrent("test 1", () => { + 8 | console.log("start test 1"); + 9 | }); + 10 | test.concurrent("test 2", () => { + 11 | throw new Error("test 2 error"); + ^ + error: test 2 error + at (file:NN:NN) + (fail) test 2 + (pass) test 3 + + 2 pass + 1 fail + Ran 3 tests across 1 file." + `); +}); diff --git a/test/js/bun/test/concurrent_immediate_error.fixture.ts b/test/js/bun/test/concurrent_immediate_error.fixture.ts new file mode 100644 index 00000000000..a318c76260b --- /dev/null +++ b/test/js/bun/test/concurrent_immediate_error.fixture.ts @@ -0,0 +1,15 @@ +beforeEach(() => { + console.log("beforeEach"); +}); +afterEach(() => { + console.log("afterEach"); +}); +test.concurrent("test 1", () => { + console.log("start test 1"); +}); +test.concurrent("test 2", () => { + throw new Error("test 2 error"); +}); +test.concurrent("test 3", () => { + console.log("start test 3"); +}); From 4246e1c9c670724a7826559c57990828039ca5fb Mon Sep 17 00:00:00 2001 From: pfg Date: Tue, 23 Sep 2025 21:26:07 -0700 Subject: [PATCH 2/7] concurrent promise fix --- src/bun.js/test/bun_test.zig | 26 +++++++-- test/js/bun/test/concurrent_immediate.test.ts | 54 ++++++++++++------- ...current_immediate_error_promise.fixture.ts | 15 ++++++ .../concurrent_immediate_promise.fixture.ts | 15 ++++++ 4 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 test/js/bun/test/concurrent_immediate_error_promise.fixture.ts create mode 100644 test/js/bun/test/concurrent_immediate_promise.fixture.ts diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index 71f5bb6bd46..b4dc1b079b4 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -328,6 +328,7 @@ pub const BunTest = struct { errdefer group.log("ended in error", .{}); const result, const this_ptr = callframe.argumentsAsArray(2); + if (this_ptr.asPtrAddress() == 0) return; // extra handler const refdata: *RefData = this_ptr.asPromisePtr(RefData); defer refdata.deref(); @@ -581,13 +582,30 @@ pub const BunTest = struct { } else bun.debugAssert(false); // this should be unreachable, we create DoneCallback above } - if (result != null and result.?.asPromise() != null) { + if (result) |result_jsvalue| if (result_jsvalue.asPromise()) |promise| { + defer result_jsvalue.ensureStillAlive(); // because sometimes we use promise without result + group.log("callTestCallback -> promise: data {}", .{cfg_data}); + // TODO: supress unhandled rejection error + result_jsvalue.then(globalThis, null, bunTestThen, bunTestCatch); + drain(globalThis); // drain microtasks to see if the promise is immediately resolved + if (dcb_ref == null) { + const immediate_status = promise.status(globalThis.vm()); + if (immediate_status != .pending) { + // immediately resolved + if (immediate_status == .rejected) { + // post error + const value = promise.result(globalThis.vm()); + this.onUncaughtException(globalThis, value, true, cfg_data); + } + return cfg_data; + } + } + // not immediately resolved; register 'then' to handle the result when it becomes available const this_ref: *RefData = if (dcb_ref) |dcb_ref_value| dcb_ref_value.dupe() else ref(this_strong, cfg_data); - result.?.then(globalThis, this_ref, bunTestThen, bunTestCatch); - drain(globalThis); + result_jsvalue.then(globalThis, this_ref, bunTestThen, bunTestCatch); return null; - } + }; if (dcb_ref) |_| { // completed asynchronously diff --git a/test/js/bun/test/concurrent_immediate.test.ts b/test/js/bun/test/concurrent_immediate.test.ts index c9bf7666dde..40256fb48f1 100644 --- a/test/js/bun/test/concurrent_immediate.test.ts +++ b/test/js/bun/test/concurrent_immediate.test.ts @@ -1,7 +1,7 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, normalizeBunSnapshot } from "harness"; -test("describe/test", async () => { +test("concurrent immediate", async () => { const result = await Bun.spawn({ cmd: [bunExe(), "test", import.meta.dir + "/concurrent_immediate.fixture.ts"], stdout: "pipe", @@ -24,9 +24,29 @@ test("describe/test", async () => { start test 3 afterEach" `); + + const result2 = await Bun.spawn({ + cmd: [bunExe(), "test", import.meta.dir + "/concurrent_immediate_promise.fixture.ts"], + stdout: "pipe", + stderr: "pipe", + env: bunEnv, + }); + const exitCode2 = await result2.exited; + const stdout2 = await result2.stdout.text(); + const stderr2 = await result2.stderr.text(); + expect(exitCode2).toBe(0); + expect(normalizeBunSnapshot(stdout2)).toBe(normalizeBunSnapshot(stdout)); + expect(normalizeBunSnapshot(stderr2).replaceAll("_promise.", ".")).toBe(normalizeBunSnapshot(stderr)); }); -test("describe/test", async () => { +function filterImportantLines(stderr: string) { + return normalizeBunSnapshot(stderr) + .split("\n") + .filter(l => l.startsWith("(pass)") || l.startsWith("(fail)") || l.startsWith("error:")) + .join("\n"); +} + +test("concurrent immediate error", async () => { const result = await Bun.spawn({ cmd: [bunExe(), "test", import.meta.dir + "/concurrent_immediate_error.fixture.ts"], stdout: "pipe", @@ -37,23 +57,21 @@ test("describe/test", async () => { const stdout = await result.stdout.text(); const stderr = await result.stderr.text(); expect(exitCode).toBe(1); - expect(normalizeBunSnapshot(stderr)).toMatchInlineSnapshot(` - "test/js/bun/test/concurrent_immediate_error.fixture.ts: - (pass) test 1 - 6 | }); - 7 | test.concurrent("test 1", () => { - 8 | console.log("start test 1"); - 9 | }); - 10 | test.concurrent("test 2", () => { - 11 | throw new Error("test 2 error"); - ^ + expect(filterImportantLines(stderr)).toMatchInlineSnapshot(` + "(pass) test 1 error: test 2 error - at (file:NN:NN) (fail) test 2 - (pass) test 3 + (pass) test 3" + `); - 2 pass - 1 fail - Ran 3 tests across 1 file." - `); + const result2 = await Bun.spawn({ + cmd: [bunExe(), "test", import.meta.dir + "/concurrent_immediate_error_promise.fixture.ts"], + stdout: "pipe", + stderr: "pipe", + env: bunEnv, + }); + const exitCode2 = await result2.exited; + const stdout2 = await result2.stdout.text(); + const stderr2 = await result2.stderr.text(); + expect(filterImportantLines(stderr2)).toBe(filterImportantLines(stderr)); }); diff --git a/test/js/bun/test/concurrent_immediate_error_promise.fixture.ts b/test/js/bun/test/concurrent_immediate_error_promise.fixture.ts new file mode 100644 index 00000000000..a757ec4fe59 --- /dev/null +++ b/test/js/bun/test/concurrent_immediate_error_promise.fixture.ts @@ -0,0 +1,15 @@ +beforeEach(async () => { + console.log("beforeEach"); +}); +afterEach(async () => { + console.log("afterEach"); +}); +test.concurrent("test 1", async () => { + console.log("start test 1"); +}); +test.concurrent("test 2", async () => { + throw new Error("test 2 error"); +}); +test.concurrent("test 3", async () => { + console.log("start test 3"); +}); diff --git a/test/js/bun/test/concurrent_immediate_promise.fixture.ts b/test/js/bun/test/concurrent_immediate_promise.fixture.ts new file mode 100644 index 00000000000..709ce5d42aa --- /dev/null +++ b/test/js/bun/test/concurrent_immediate_promise.fixture.ts @@ -0,0 +1,15 @@ +beforeEach(async () => { + console.log("beforeEach"); +}); +afterEach(async () => { + console.log("afterEach"); +}); +test.concurrent("test 1", async () => { + console.log("start test 1"); +}); +test.concurrent("test 2", async () => { + console.log("start test 2"); +}); +test.concurrent("test 3", async () => { + console.log("start test 3"); +}); From b537feed04488306507fdfb1c282f17b4d90781e Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 24 Sep 2025 00:35:40 -0700 Subject: [PATCH 3/7] Fix timeouts --- src/bun.js/test/Collection.zig | 2 +- src/bun.js/test/Execution.zig | 26 ++++++------ src/bun.js/test/bun_test.zig | 73 +++++++++++++++++++++++----------- 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/src/bun.js/test/Collection.zig b/src/bun.js/test/Collection.zig index 10e5422b2a1..b4e24054361 100644 --- a/src/bun.js/test/Collection.zig +++ b/src/bun.js/test/Collection.zig @@ -139,7 +139,7 @@ pub fn step(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject if (BunTest.runTestCallback(buntest_strong, globalThis, callback.get(), false, .{ .collection = .{ .active_scope = previous_scope }, - }, .epoch)) |cfg_data| { + }, &.epoch)) |cfg_data| { // the result is available immediately; queue buntest.addResult(cfg_data); } diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index eb0b99a7e20..c57ae6414c9 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -222,10 +222,11 @@ pub fn step(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject defer groupLog.end(); const buntest = buntest_strong.get(); const this = &buntest.execution; + var now = bun.timespec.now(); switch (data) { .start => { - return try stepGroup(buntest_strong, globalThis, bun.timespec.now()); + return try stepGroup(buntest_strong, globalThis, &now); }, else => { // determine the active sequence,group @@ -242,21 +243,20 @@ pub fn step(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject bun.assert(sequence.active_index < sequence.entries(this).len); this.advanceSequence(sequence, group); - const now = bun.timespec.now(); - const sequence_result = try stepSequence(buntest_strong, globalThis, sequence, group, sequence_index, now); + const sequence_result = try stepSequence(buntest_strong, globalThis, sequence, group, sequence_index, &now); switch (sequence_result) { .done => {}, .execute => |exec| return .{ .waiting = .{ .timeout = exec.timeout } }, } if (group.remaining_incomplete_entries == 0) { - return try stepGroup(buntest_strong, globalThis, now); + return try stepGroup(buntest_strong, globalThis, &now); } return .{ .waiting = .{} }; }, } } -pub fn stepGroup(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, now: bun.timespec) bun.JSError!bun_test.StepResult { +pub fn stepGroup(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, now: *bun.timespec) bun.JSError!bun_test.StepResult { groupLog.begin(@src()); defer groupLog.end(); const buntest = buntest_strong.get(); @@ -295,7 +295,7 @@ pub fn stepGroup(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalO } } const AdvanceStatus = union(enum) { done, execute: struct { timeout: bun.timespec = .epoch } }; -fn stepGroupOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, group: *ConcurrentGroup, now: bun.timespec) !AdvanceStatus { +fn stepGroupOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, group: *ConcurrentGroup, now: *bun.timespec) !AdvanceStatus { const buntest = buntest_strong.get(); const this = &buntest.execution; var final_status: AdvanceStatus = .done; @@ -320,13 +320,13 @@ const AdvanceSequenceStatus = union(enum) { timeout: bun.timespec = .epoch, }, }; -fn stepSequence(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, sequence: *ExecutionSequence, group: *ConcurrentGroup, sequence_index: usize, now: bun.timespec) !AdvanceSequenceStatus { +fn stepSequence(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, sequence: *ExecutionSequence, group: *ConcurrentGroup, sequence_index: usize, now: *bun.timespec) !AdvanceSequenceStatus { while (true) { return try stepSequenceOne(buntest_strong, globalThis, sequence, group, sequence_index, now) orelse continue; } } /// returns null if the while loop should continue -fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, sequence: *ExecutionSequence, group: *ConcurrentGroup, sequence_index: usize, now: bun.timespec) !?AdvanceSequenceStatus { +fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject, sequence: *ExecutionSequence, group: *ConcurrentGroup, sequence_index: usize, now: *bun.timespec) !?AdvanceSequenceStatus { groupLog.begin(@src()); defer groupLog.end(); const buntest = buntest_strong.get(); @@ -337,10 +337,7 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba bun.debugAssert(false); // sequence is executing with no active entry return .{ .execute = .{} }; }; - if (!active_entry.timespec.eql(&.epoch) and active_entry.timespec.order(&now) == .lt) { - // timed out - sequence.result = if (active_entry == sequence.test_entry) if (active_entry.has_done_parameter) .fail_because_timeout_with_done_callback else .fail_because_timeout else if (active_entry.has_done_parameter) .fail_because_hook_timeout_with_done_callback else .fail_because_hook_timeout; - sequence.maybe_skip = true; + if (active_entry.evaluateTimeout(sequence, now)) { this.advanceSequence(sequence, group); return null; // run again } @@ -374,7 +371,10 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba }; groupLog.log("runSequence queued callback: {}", .{callback_data}); - if (BunTest.runTestCallback(buntest_strong, globalThis, cb.get(), next_item.has_done_parameter, callback_data, next_item.timespec) != null) { + if (BunTest.runTestCallback(buntest_strong, globalThis, cb.get(), next_item.has_done_parameter, callback_data, &next_item.timespec) != null) { + now.* = bun.timespec.now(); + _ = next_item.evaluateTimeout(sequence, now); + // the result is available immediately; advance the sequence and run again. this.advanceSequence(sequence, group); return null; // run again diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index c52dd17dd0e..0b2d017bc51 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -328,7 +328,7 @@ pub const BunTest = struct { errdefer group.log("ended in error", .{}); const result, const this_ptr = callframe.argumentsAsArray(2); - if (this_ptr.asPtrAddress() == 0) return; // extra handler + if (this_ptr.isEmptyOrUndefinedOrNull()) return; const refdata: *RefData = this_ptr.asPromisePtr(RefData); defer refdata.deref(); @@ -470,21 +470,21 @@ pub const BunTest = struct { } } - this.updateMinTimeout(globalThis, min_timeout); + this.updateMinTimeout(globalThis, &min_timeout); } - fn updateMinTimeout(this: *BunTest, globalThis: *jsc.JSGlobalObject, min_timeout: bun.timespec) void { + fn updateMinTimeout(this: *BunTest, globalThis: *jsc.JSGlobalObject, min_timeout: *const bun.timespec) void { group.begin(@src()); defer group.end(); // only set the timer if the new timeout is sooner than the current timeout. this unfortunately means that we can't unset an unnecessary timer. - group.log("-> timeout: {} {}, {s}", .{ min_timeout, this.timer.next, @tagName(min_timeout.orderIgnoreEpoch(this.timer.next)) }); + group.log("-> timeout: {} {}, {s}", .{ min_timeout.*, this.timer.next, @tagName(min_timeout.orderIgnoreEpoch(this.timer.next)) }); if (min_timeout.orderIgnoreEpoch(this.timer.next) == .lt) { - group.log("-> setting timer to {}", .{min_timeout}); + group.log("-> setting timer to {}", .{min_timeout.*}); if (!this.timer.next.eql(&.epoch)) { group.log("-> removing existing timer", .{}); globalThis.bunVM().timer.remove(&this.timer); } - this.timer.next = min_timeout; + this.timer.next = min_timeout.*; if (!this.timer.next.eql(&.epoch)) { group.log("-> inserting timer", .{}); globalThis.bunVM().timer.insert(&this.timer); @@ -545,7 +545,7 @@ pub const BunTest = struct { } /// if sync, the result is returned. if async, null is returned. - pub fn runTestCallback(this_strong: BunTestPtr, globalThis: *jsc.JSGlobalObject, cfg_callback: jsc.JSValue, cfg_done_parameter: bool, cfg_data: BunTest.RefDataValue, timeout: bun.timespec) ?RefDataValue { + pub fn runTestCallback(this_strong: BunTestPtr, globalThis: *jsc.JSGlobalObject, cfg_callback: jsc.JSValue, cfg_done_parameter: bool, cfg_data: BunTest.RefDataValue, timeout: *const bun.timespec) ?RefDataValue { group.begin(@src()); defer group.end(); const this = this_strong.get(); @@ -586,25 +586,30 @@ pub const BunTest = struct { defer result_jsvalue.ensureStillAlive(); // because sometimes we use promise without result group.log("callTestCallback -> promise: data {}", .{cfg_data}); - // TODO: supress unhandled rejection error - result_jsvalue.then(globalThis, null, bunTestThen, bunTestCatch); - drain(globalThis); // drain microtasks to see if the promise is immediately resolved - if (dcb_ref == null) { - const immediate_status = promise.status(globalThis.vm()); - if (immediate_status != .pending) { - // immediately resolved - if (immediate_status == .rejected) { - // post error - const value = promise.result(globalThis.vm()); - this.onUncaughtException(globalThis, value, true, cfg_data); - } + const vm = globalThis.bunVM(); + + vm.drainMicrotasks(); + + switch (promise.status(globalThis.vm())) { + .pending => { + // not immediately resolved; register 'then' to handle the result when it becomes available + const this_ref: *RefData = if (dcb_ref) |dcb_ref_value| dcb_ref_value.dupe() else ref(this_strong, cfg_data); + result_jsvalue.then(globalThis, this_ref, bunTestThen, bunTestCatch); + return null; + }, + .fulfilled => { + // Do not register a then callback when it's already fulfilled. return cfg_data; - } + }, + .rejected => { + // Prevent the promise from entering the promise rejection queue. + promise.setHandled(globalThis.vm()); + + const value = promise.result(globalThis.vm()); + this.onUncaughtException(globalThis, value, true, cfg_data); + return cfg_data; + }, } - // not immediately resolved; register 'then' to handle the result when it becomes available - const this_ref: *RefData = if (dcb_ref) |dcb_ref_value| dcb_ref_value.dupe() else ref(this_strong, cfg_data); - result_jsvalue.then(globalThis, this_ref, bunTestThen, bunTestCatch); - return null; }; if (dcb_ref) |_| { @@ -857,6 +862,26 @@ pub const ExecutionEntry = struct { } return entry; } + + pub fn evaluateTimeout(this: *ExecutionEntry, sequence: *Execution.ExecutionSequence, now: *const bun.timespec) bool { + if (!this.timespec.eql(&.epoch) and this.timespec.order(now) == .lt) { + // timed out + sequence.result = if (this == sequence.test_entry) + if (this.has_done_parameter) + .fail_because_timeout_with_done_callback + else + .fail_because_timeout + else if (this.has_done_parameter) + .fail_because_hook_timeout_with_done_callback + else + .fail_because_hook_timeout; + sequence.maybe_skip = true; + return true; + } + + return false; + } + pub fn destroy(this: *ExecutionEntry, gpa: std.mem.Allocator) void { if (this.callback) |*c| c.deinit(); this.base.deinit(gpa); From 4fc0d65cb032c1ef91dcbab4b989178775ce032b Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 24 Sep 2025 01:20:03 -0700 Subject: [PATCH 4/7] Clean up a little bit --- src/bun.js/event_loop.zig | 8 +++ src/bun.js/test/bun_test.zig | 98 +++++++++++++++++++----------------- 2 files changed, 59 insertions(+), 47 deletions(-) diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index 58b149db962..dd4451e81f1 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -182,6 +182,14 @@ comptime { @export(&externRunCallback3, .{ .name = "Bun__EventLoop__runCallback3" }); } +/// Prefer `runCallbackWithResult` unless you really need to make sure that microtasks are drained. +pub fn runCallbackWithResultAndForcefullyDrainMicrotasks(this: *EventLoop, callback: jsc.JSValue, globalObject: *jsc.JSGlobalObject, thisValue: jsc.JSValue, arguments: []const jsc.JSValue) !jsc.JSValue { + const result = try callback.call(globalObject, thisValue, arguments); + result.ensureStillAlive(); + try this.drainMicrotasksWithGlobal(globalObject, globalObject.bunVM().jsc_vm); + return result; +} + pub fn runCallbackWithResult(this: *EventLoop, callback: jsc.JSValue, globalObject: *jsc.JSGlobalObject, thisValue: jsc.JSValue, arguments: []const jsc.JSValue) jsc.JSValue { this.enter(); defer this.exit(); diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index 0b2d017bc51..6570c9609b5 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -532,48 +532,55 @@ pub const BunTest = struct { } } - fn drain(globalThis: *jsc.JSGlobalObject) void { - const bun_vm = globalThis.bunVM(); - bun_vm.drainMicrotasks(); - var count = bun_vm.unhandled_error_counter; - bun_vm.global.handleRejectedPromises(); - while (bun_vm.unhandled_error_counter > count) { - count = bun_vm.unhandled_error_counter; - bun_vm.drainMicrotasks(); - bun_vm.global.handleRejectedPromises(); - } - } - /// if sync, the result is returned. if async, null is returned. pub fn runTestCallback(this_strong: BunTestPtr, globalThis: *jsc.JSGlobalObject, cfg_callback: jsc.JSValue, cfg_done_parameter: bool, cfg_data: BunTest.RefDataValue, timeout: *const bun.timespec) ?RefDataValue { group.begin(@src()); defer group.end(); const this = this_strong.get(); + const vm = globalThis.bunVM(); - var done_arg: ?jsc.JSValue = null; + // Don't use ?jsc.JSValue to make it harder for the conservative stack + // scanner to miss it. + var done_arg: jsc.JSValue = .zero; + var done_callback: jsc.JSValue = .zero; - var done_callback: ?jsc.JSValue = null; if (cfg_done_parameter) { group.log("callTestCallback -> appending done callback param: data {}", .{cfg_data}); done_callback = DoneCallback.createUnbound(globalThis); - done_arg = DoneCallback.bind(done_callback.?, globalThis) catch |e| blk: { + done_arg = DoneCallback.bind(done_callback, globalThis) catch |e| blk: { this.onUncaughtException(globalThis, globalThis.takeException(e), false, cfg_data); - break :blk jsc.JSValue.js_undefined; // failed to bind done callback + break :blk .zero; // failed to bind done callback }; } this.updateMinTimeout(globalThis, timeout); - const result: ?jsc.JSValue = cfg_callback.call(globalThis, .js_undefined, if (done_arg) |done| &.{done} else &.{}) catch blk: { + const result: jsc.JSValue = vm.eventLoop().runCallbackWithResultAndForcefullyDrainMicrotasks(cfg_callback, globalThis, .js_undefined, if (done_arg != .zero) &.{done_arg} else &.{}) catch blk: { globalThis.clearTerminationException(); this.onUncaughtException(globalThis, globalThis.tryTakeException(), false, cfg_data); group.log("callTestCallback -> error", .{}); - break :blk null; + break :blk .zero; }; + done_callback.ensureStillAlive(); + + // Drain unhandled promise rejections. + while (true) { + // Prevent the user's Promise rejection from going into the uncaught promise rejection queue. + if (result != .zero) + if (result.asPromise()) |promise| + if (promise.status(globalThis.vm()) == .rejected) + promise.setHandled(globalThis.vm()); + + const prev_unhandled_count = vm.unhandled_error_counter; + globalThis.handleRejectedPromises(); + if (vm.unhandled_error_counter == prev_unhandled_count) + break; + } + var dcb_ref: ?*RefData = null; - if (done_callback) |dcb| { - if (DoneCallback.fromJS(dcb)) |dcb_data| { - if (dcb_data.called or result == null) { + if (done_callback != .zero and result != .zero) { + if (DoneCallback.fromJS(done_callback)) |dcb_data| { + if (dcb_data.called) { // done callback already called or the callback errored; add result immediately } else { dcb_ref = ref(this_strong, cfg_data); @@ -582,45 +589,42 @@ pub const BunTest = struct { } else bun.debugAssert(false); // this should be unreachable, we create DoneCallback above } - if (result) |result_jsvalue| if (result_jsvalue.asPromise()) |promise| { - defer result_jsvalue.ensureStillAlive(); // because sometimes we use promise without result + if (result != .zero) { + if (result.asPromise()) |promise| { + defer result.ensureStillAlive(); // because sometimes we use promise without result - group.log("callTestCallback -> promise: data {}", .{cfg_data}); - const vm = globalThis.bunVM(); + group.log("callTestCallback -> promise: data {}", .{cfg_data}); - vm.drainMicrotasks(); + switch (promise.status(globalThis.vm())) { + .pending => { + // not immediately resolved; register 'then' to handle the result when it becomes available + const this_ref: *RefData = if (dcb_ref) |dcb_ref_value| dcb_ref_value.dupe() else ref(this_strong, cfg_data); + result.then(globalThis, this_ref, bunTestThen, bunTestCatch); + return null; + }, + .fulfilled => { + // Do not register a then callback when it's already fulfilled. + return cfg_data; + }, + .rejected => { + const value = promise.result(globalThis.vm()); + this.onUncaughtException(globalThis, value, true, cfg_data); - switch (promise.status(globalThis.vm())) { - .pending => { - // not immediately resolved; register 'then' to handle the result when it becomes available - const this_ref: *RefData = if (dcb_ref) |dcb_ref_value| dcb_ref_value.dupe() else ref(this_strong, cfg_data); - result_jsvalue.then(globalThis, this_ref, bunTestThen, bunTestCatch); - return null; - }, - .fulfilled => { - // Do not register a then callback when it's already fulfilled. - return cfg_data; - }, - .rejected => { - // Prevent the promise from entering the promise rejection queue. - promise.setHandled(globalThis.vm()); + // We previously marked it as handled above. - const value = promise.result(globalThis.vm()); - this.onUncaughtException(globalThis, value, true, cfg_data); - return cfg_data; - }, + return cfg_data; + }, + } } - }; + } if (dcb_ref) |_| { // completed asynchronously group.log("callTestCallback -> wait for done callback", .{}); - drain(globalThis); return null; } group.log("callTestCallback -> sync", .{}); - drain(globalThis); return cfg_data; } From de9018391895ca130dae16589241ffe56cfca9b3 Mon Sep 17 00:00:00 2001 From: pfg Date: Wed, 24 Sep 2025 20:23:32 -0700 Subject: [PATCH 5/7] remove synchronous timeout check --- src/bun.js/test/Execution.zig | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index c57ae6414c9..fee994660f9 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -373,7 +373,6 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba if (BunTest.runTestCallback(buntest_strong, globalThis, cb.get(), next_item.has_done_parameter, callback_data, &next_item.timespec) != null) { now.* = bun.timespec.now(); - _ = next_item.evaluateTimeout(sequence, now); // the result is available immediately; advance the sequence and run again. this.advanceSequence(sequence, group); From c4d565b8db03bef177534bf7fdfd09dfeba0f5bb Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 25 Sep 2025 02:31:39 -0700 Subject: [PATCH 6/7] Revert "remove synchronous timeout check" This reverts commit de9018391895ca130dae16589241ffe56cfca9b3. --- src/bun.js/test/Execution.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index fee994660f9..c57ae6414c9 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -373,6 +373,7 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba if (BunTest.runTestCallback(buntest_strong, globalThis, cb.get(), next_item.has_done_parameter, callback_data, &next_item.timespec) != null) { now.* = bun.timespec.now(); + _ = next_item.evaluateTimeout(sequence, now); // the result is available immediately; advance the sequence and run again. this.advanceSequence(sequence, group); From f1174252770a681ae38e0898b452a7ae6374cf3b Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 25 Sep 2025 02:41:09 -0700 Subject: [PATCH 7/7] Update fs.test.ts --- test/js/node/fs/fs.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index c56483180a5..f4196f282f2 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -13,6 +13,7 @@ import fs, { fdatasync, fdatasyncSync, fstatSync, + ftruncateSync, lstatSync, mkdirSync, mkdtemp, @@ -2714,14 +2715,15 @@ it("fstat on a large file", () => { try { dest = `${tmpdir()}/fs.test.ts/${Math.trunc(Math.random() * 10000000000).toString(32)}.stat.txt`; mkdirSync(dirname(dest), { recursive: true }); - const bigBuffer = new Uint8Array(1024 * 1024 * 1024); fd = openSync(dest, "w"); - let offset = 0; - while (offset < 5 * 1024 * 1024 * 1024) { - offset += writeSync(fd, bigBuffer, 0, bigBuffer.length, offset); - } + + // Instead of writing the actual bytes, we can use ftruncate to make a + // hole-y file and extend it to the desired size This should generally avoid + // the ENOSPC issue and avoid timeouts. + ftruncateSync(fd, 5 * 1024 * 1024 * 1024); fdatasyncSync(fd); - expect(fstatSync(fd).size).toEqual(offset); + const stats = fstatSync(fd); + expect(stats.size).toEqual(5 * 1024 * 1024 * 1024); } catch (error) { // TODO: Once `fs.statfsSync` is implemented, make sure that the buffer size // is small enough not to cause: ENOSPC: No space left on device.