From 808583cccbe28a7b81560503378b94c73d0f9cfa Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 29 Sep 2025 14:45:33 -0700 Subject: [PATCH 01/11] wip.1 --- TODO.md | 1 + src/bun.js/test/Execution.zig | 97 +++++++++++++----------------- src/bun.js/test/Order.zig | 64 +++++++++++--------- src/bun.js/test/ScopeFunctions.zig | 2 +- src/bun.js/test/bun_test.zig | 39 ++++++++---- src/bun.js/test/debug.zig | 5 +- 6 files changed, 109 insertions(+), 99 deletions(-) create mode 100644 TODO.md diff --git a/TODO.md b/TODO.md new file mode 100644 index 00000000000..07c8381d4cd --- /dev/null +++ b/TODO.md @@ -0,0 +1 @@ +- [ ] revert changes made to node:test implementation diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index 2c02034bd60..a44249965ec 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -38,7 +38,6 @@ groups: []ConcurrentGroup, #sequences: []ExecutionSequence, /// the entries themselves are owned by BunTest, which owns Execution. -#entries: []const *ExecutionEntry, group_index: usize, pub const ConcurrentGroup = struct { @@ -73,8 +72,9 @@ pub const ConcurrentGroup = struct { } }; pub const ExecutionSequence = struct { + first_entry: ?*ExecutionEntry, /// Index into ExecutionSequence.entries() for the entry that is not started or currently running - active_index: usize, + active_entry: ?*ExecutionEntry, test_entry: ?*ExecutionEntry, remaining_repeat_count: i64 = 1, result: Result = .pending, @@ -90,16 +90,10 @@ pub const ExecutionSequence = struct { } = .not_set, maybe_skip: bool = false, - /// Start index into `Execution.#entries` (inclusive) for this sequence. - #entries_start: usize, - /// End index into `Execution.#entries` (exclusive) for this sequence. - #entries_end: usize, - - pub fn init(start: usize, end: usize, test_entry: ?*ExecutionEntry) ExecutionSequence { + pub fn init(first_entry: ?*ExecutionEntry, test_entry: ?*ExecutionEntry) ExecutionSequence { return .{ - .#entries_start = start, - .#entries_end = end, - .active_index = 0, + .first_entry = first_entry, + .active_entry = first_entry, .test_entry = test_entry, }; } @@ -108,15 +102,6 @@ pub const ExecutionSequence = struct { if (this.test_entry) |entry| return entry.base.mode; return .normal; } - - pub fn entries(this: ExecutionSequence, execution: *Execution) []const *ExecutionEntry { - return execution.#entries[this.#entries_start..this.#entries_end]; - } - pub fn activeEntry(this: ExecutionSequence, execution: *Execution) ?*ExecutionEntry { - const entries_value = this.entries(execution); - if (this.active_index >= entries_value.len) return null; - return entries_value[this.active_index]; - } }; pub const Result = enum { pending, @@ -166,26 +151,21 @@ pub fn init(_: std.mem.Allocator) Execution { return .{ .groups = &.{}, .#sequences = &.{}, - .#entries = &.{}, .group_index = 0, }; } pub fn deinit(this: *Execution) void { this.bunTest().gpa.free(this.groups); this.bunTest().gpa.free(this.#sequences); - this.bunTest().gpa.free(this.#entries); } pub fn loadFromOrder(this: *Execution, order: *Order) bun.JSError!void { bun.assert(this.groups.len == 0); bun.assert(this.#sequences.len == 0); - bun.assert(this.#entries.len == 0); var alloc_safety = bun.safety.CheckedAllocator.init(this.bunTest().gpa); alloc_safety.assertEq(order.groups.allocator); alloc_safety.assertEq(order.sequences.allocator); - alloc_safety.assertEq(order.entries.allocator); this.groups = try order.groups.toOwnedSlice(); this.#sequences = try order.sequences.toOwnedSlice(); - this.#entries = try order.entries.toOwnedSlice(); } fn bunTest(this: *Execution) *BunTest { @@ -203,7 +183,7 @@ pub fn handleTimeout(this: *Execution, globalThis: *jsc.JSGlobalObject) bun.JSEr const sequences = current_group.sequences(this); if (sequences.len == 1) { const sequence = sequences[0]; - if (sequence.activeEntry(this)) |entry| { + if (sequence.active_entry) |entry| { const now = bun.timespec.now(); if (entry.timespec.order(&now) == .lt) { globalThis.requestTermination(); @@ -243,7 +223,7 @@ pub fn step(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGlobalObject }; const sequence_index = data.execution.entry_data.?.sequence_index; - bun.assert(sequence.active_index < sequence.entries(this).len); + if (bun.Environment.ci_assert) bun.assert(sequence.active_entry != null); this.advanceSequence(sequence, group); const sequence_result = try stepSequence(buntest_strong, globalThis, group, sequence_index, &now); @@ -357,7 +337,7 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba const sequence = &group.sequences(this)[sequence_index]; if (sequence.executing) { - const active_entry = sequence.activeEntry(this) orelse { + const active_entry = sequence.active_entry orelse { bun.debugAssert(false); // sequence is executing with no active entry return .{ .execute = .{} }; }; @@ -369,13 +349,13 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba return .{ .execute = .{ .timeout = active_entry.timespec } }; } - const next_item = sequence.activeEntry(this) orelse { + const next_item = sequence.active_entry orelse { bun.debugAssert(sequence.remaining_repeat_count == 0); // repeat count is decremented when the sequence is advanced, this should only happen if the sequence were empty. which should be impossible. groupLog.log("runOne: no repeats left; wait for group completion.", .{}); return .done; }; sequence.executing = true; - if (sequence.active_index == 0) { + if (next_item == sequence.first_entry) { this.onSequenceStarted(sequence); } this.onEntryStarted(next_item); @@ -388,7 +368,7 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba .group_index = this.group_index, .entry_data = .{ .sequence_index = sequence_index, - .entry_index = sequence.active_index, + .entry = next_item, .remaining_repeat_count = sequence.remaining_repeat_count, }, }, @@ -416,7 +396,7 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba sequence.result = .skipped_because_label; }, else => { - groupLog.log("runSequence: no callback for sequence_index {d} (entry_index {d})", .{ sequence_index, sequence.active_index }); + groupLog.log("runSequence: no callback for sequence_index {d} (entry_index {x})", .{ sequence_index, @intFromPtr(sequence.active_entry) }); bun.debugAssert(false); }, } @@ -458,7 +438,7 @@ fn getCurrentAndValidExecutionSequence(this: *Execution, data: bun_test.BunTest. groupLog.log("runOneCompleted: the data is for a previous repeat count (outdated)", .{}); return null; } - if (sequence.active_index != data.execution.entry_data.?.entry_index) { + if (sequence.active_entry != data.execution.entry_data.?.entry) { groupLog.log("runOneCompleted: the data is for a different sequence index (outdated)", .{}); return null; } @@ -470,27 +450,21 @@ fn advanceSequence(this: *Execution, sequence: *ExecutionSequence, group: *Concu defer groupLog.end(); bun.assert(sequence.executing); - if (sequence.activeEntry(this)) |entry| { + if (sequence.active_entry) |entry| { this.onEntryCompleted(entry); - } else { - bun.debugAssert(false); // sequence is executing with no active entry? - } - sequence.executing = false; - if (sequence.maybe_skip) { - sequence.maybe_skip = false; - const first_aftereach_index = for (sequence.entries(this), 0..) |entry, index| { - if (entry == sequence.test_entry) break index + 1; - } else sequence.entries(this).len; - if (sequence.active_index < first_aftereach_index) { - sequence.active_index = first_aftereach_index; + + sequence.executing = false; + if (sequence.maybe_skip) { + sequence.maybe_skip = false; + sequence.active_entry = entry.skip_to; } else { - sequence.active_index = sequence.entries(this).len; + sequence.active_entry = entry.next; } } else { - sequence.active_index += 1; + if (bun.Environment.ci_assert) bun.assert(false); // can't call advanceSequence on a completed sequence } - if (sequence.activeEntry(this) == null) { + if (sequence.active_entry == null) { // just completed the sequence this.onSequenceCompleted(sequence); sequence.remaining_repeat_count -= 1; @@ -561,10 +535,9 @@ fn onSequenceCompleted(this: *Execution, sequence: *ExecutionSequence) void { else => .pass, }; } - const entries = sequence.entries(this); - if (entries.len > 0 and (sequence.test_entry != null or sequence.result != .pass)) { - test_command.CommandLineReporter.handleTestCompleted(this.bunTest(), sequence, sequence.test_entry orelse entries[0], elapsed_ns); - } + if (sequence.first_entry) |first_entry| if (sequence.test_entry != null or sequence.result != .pass) { + test_command.CommandLineReporter.handleTestCompleted(this.bunTest(), sequence, sequence.test_entry orelse first_entry, elapsed_ns); + }; if (sequence.test_entry) |entry| { if (entry.base.test_id_for_debugger != 0) { @@ -593,13 +566,27 @@ fn onSequenceCompleted(this: *Execution, sequence: *ExecutionSequence) void { } pub fn resetSequence(this: *Execution, sequence: *ExecutionSequence) void { bun.assert(!sequence.executing); + { + // reset the entries + var current_entry = sequence.first_entry; + while (current_entry) |entry| : (current_entry = entry.next) { + // remove entries that were added in the execution phase + while (entry.next != null and entry.next.?.added_in_phase == .execution) { + entry.next = entry.next.?.next; + // can't deinit the removed entry because it may still be referenced in a RefDataValue + } + entry.timespec = .epoch; + } + } + if (sequence.result.isPass(.pending_is_pass)) { // passed or pending; run again - sequence.* = .init(sequence.#entries_start, sequence.#entries_end, sequence.test_entry); + sequence.* = .init(sequence.first_entry, sequence.test_entry); } else { // already failed or skipped; don't run again - sequence.active_index = sequence.entries(this).len; + sequence.active_entry = null; } + _ = this; } pub fn handleUncaughtException(this: *Execution, user_data: bun_test.BunTest.RefDataValue) bun_test.HandleUncaughtExceptionResult { @@ -612,7 +599,7 @@ pub fn handleUncaughtException(this: *Execution, user_data: bun_test.BunTest.Ref _ = group; sequence.maybe_skip = true; - if (sequence.activeEntry(this) != sequence.test_entry) { + if (sequence.active_entry != sequence.test_entry) { // executing hook if (sequence.result == .pending) sequence.result = .fail; return .show_handled_error; diff --git a/src/bun.js/test/Order.zig b/src/bun.js/test/Order.zig index 9f220fafc84..0ff069aa6b6 100644 --- a/src/bun.js/test/Order.zig +++ b/src/bun.js/test/Order.zig @@ -2,20 +2,17 @@ groups: std.ArrayList(ConcurrentGroup), sequences: std.ArrayList(ExecutionSequence), -entries: std.ArrayList(*ExecutionEntry), previous_group_was_concurrent: bool = false, pub fn init(gpa: std.mem.Allocator) Order { return .{ .groups = std.ArrayList(ConcurrentGroup).init(gpa), .sequences = std.ArrayList(ExecutionSequence).init(gpa), - .entries = std.ArrayList(*ExecutionEntry).init(gpa), }; } pub fn deinit(this: *Order) void { this.groups.deinit(); this.sequences.deinit(); - this.entries.deinit(); } pub fn generateOrderSub(this: *Order, current: TestScheduleEntry, cfg: Config) bun.JSError!void { @@ -43,11 +40,10 @@ pub const Config = struct { pub fn generateAllOrder(this: *Order, entries: []const *ExecutionEntry, _: Config) bun.JSError!AllOrderResult { const start = this.groups.items.len; for (entries) |entry| { - const entries_start = this.entries.items.len; - try this.entries.append(entry); // add entry to sequence - const entries_end = this.entries.items.len; + entry.next = null; + entry.skip_to = null; const sequences_start = this.sequences.items.len; - try this.sequences.append(.init(entries_start, entries_end, null)); // add sequence to concurrentgroup + try this.sequences.append(.init(entry, null)); // add sequence to concurrentgroup const sequences_end = this.sequences.items.len; try this.groups.append(.init(sequences_start, sequences_end, this.groups.items.len + 1)); // add a new concurrentgroup to order this.previous_group_was_concurrent = false; @@ -82,48 +78,60 @@ pub fn generateOrderDescribe(this: *Order, current: *DescribeScope, cfg: Config) // update skip_to values for afterAll to skip the remaining afterAll items afterall_order.setFailureSkipTo(this); } + +const EntryList = struct { + first: ?*ExecutionEntry = null, + last: ?*ExecutionEntry = null, + pub fn append(this: *EntryList, current: *ExecutionEntry) void { + if (this.last) |last| { + last.next = current; + this.last = current; + } else { + this.first = current; + this.last = current; + } + } +}; pub fn generateOrderTest(this: *Order, current: *ExecutionEntry, _: Config) bun.JSError!void { - const entries_start = this.entries.items.len; bun.assert(current.base.has_callback == (current.callback != null)); const use_each_hooks = current.base.has_callback; + var list: EntryList = .{}; + // gather beforeEach (alternatively, this could be implemented recursively to make it less complicated) if (use_each_hooks) { - // determine length of beforeEach - var beforeEachLen: usize = 0; - { - var parent: ?*DescribeScope = current.base.parent; - while (parent) |p| : (parent = p.base.parent) { - beforeEachLen += p.beforeEach.items.len; - } - } - // copy beforeEach entries - const beforeEachSlice = try this.entries.addManyAsSlice(beforeEachLen); // add entries to sequence - { - var parent: ?*DescribeScope = current.base.parent; - var i: usize = beforeEachLen; - while (parent) |p| : (parent = p.base.parent) { - i -= p.beforeEach.items.len; - @memcpy(beforeEachSlice[i..][0..p.beforeEach.items.len], p.beforeEach.items); + var parent: ?*DescribeScope = current.base.parent; + while (parent) |p| : (parent = p.base.parent) { + for (p.beforeEach.items) |entry| { + list.append(entry); } } } // append test - try this.entries.append(current); // add entry to sequence + list.append(current); // add entry to sequence // gather afterEach if (use_each_hooks) { var parent: ?*DescribeScope = current.base.parent; while (parent) |p| : (parent = p.base.parent) { - try this.entries.appendSlice(p.afterEach.items); // add entry to sequence + for (p.afterEach.items) |entry| { + list.append(entry); + } } } + // set skip_to values + var index = list.first; + var skip_to = current.next; + while (index) |entry| : (index = entry.next) { + if (entry == skip_to) skip_to = null; + entry.skip_to = skip_to; + } + // add these as a single sequence - const entries_end = this.entries.items.len; const sequences_start = this.sequences.items.len; - try this.sequences.append(.init(entries_start, entries_end, current)); // add sequence to concurrentgroup + try this.sequences.append(.init(list.first, current)); // add sequence to concurrentgroup const sequences_end = this.sequences.items.len; try appendOrExtendConcurrentGroup(this, current.base.concurrent, sequences_start, sequences_end); // add or extend the concurrent group } diff --git a/src/bun.js/test/ScopeFunctions.zig b/src/bun.js/test/ScopeFunctions.zig index c7619f14de3..936aee53806 100644 --- a/src/bun.js/test/ScopeFunctions.zig +++ b/src/bun.js/test/ScopeFunctions.zig @@ -249,7 +249,7 @@ fn enqueueDescribeOrTestCallback(this: *ScopeFunctions, bunTest: *bun_test.BunTe _ = try bunTest.collection.active_scope.appendTest(bunTest.gpa, description, if (matches_filter) callback else null, .{ .has_done_parameter = has_done_parameter, .timeout = timeout, - }, base); + }, base, .collection); }, } } diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index d4a93b45c15..4aa28f01b5c 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -57,7 +57,7 @@ pub const js_fns = struct { _ = try bunTestRoot.hook_scope.appendHook(bunTestRoot.gpa, tag, args.callback, .{ .has_done_parameter = has_done_parameter, .timeout = args.options.timeout, - }, .{}); + }, .{}, .preload); return .js_undefined; }; @@ -66,7 +66,7 @@ pub const js_fns = struct { _ = try bunTest.collection.active_scope.appendHook(bunTest.gpa, tag, args.callback, .{ .has_done_parameter = has_done_parameter, .timeout = args.options.timeout, - }, .{}); + }, .{}, .collection); return .js_undefined; }, @@ -217,7 +217,7 @@ pub const BunTest = struct { group_index: usize, entry_data: ?struct { sequence_index: usize, - entry_index: usize, + entry: *ExecutionEntry, remaining_repeat_count: i64, }, }, @@ -233,11 +233,10 @@ pub const BunTest = struct { const entry_data = this.execution.entry_data orelse return null; return &group_item.sequences(&buntest.execution)[entry_data.sequence_index]; } - pub fn entry(this: *const RefDataValue, buntest: *BunTest) ?*ExecutionEntry { + pub fn entry(this: *const RefDataValue, _: *BunTest) ?*ExecutionEntry { if (this.* != .execution) return null; - const sequence_item = this.sequence(buntest) orelse return null; const entry_data = this.execution.entry_data orelse return null; - return sequence_item.entries(&buntest.execution)[entry_data.entry_index]; + return entry_data.entry; } pub fn format(this: *const RefDataValue, comptime _: []const u8, _: std.fmt.FormatOptions, writer: anytype) !void { @@ -245,7 +244,7 @@ pub const BunTest = struct { .start => try writer.print("start", .{}), .collection => try writer.print("collection: active_scope={?s}", .{this.collection.active_scope.base.name}), .execution => if (this.execution.entry_data) |entry_data| { - try writer.print("execution: group_index={d},sequence_index={d},entry_index={d},remaining_repeat_count={d}", .{ this.execution.group_index, entry_data.sequence_index, entry_data.entry_index, entry_data.remaining_repeat_count }); + try writer.print("execution: group_index={d},sequence_index={d},entry_index={x},remaining_repeat_count={d}", .{ this.execution.group_index, entry_data.sequence_index, @intFromPtr(entry_data.entry), entry_data.remaining_repeat_count }); } else try writer.print("execution: group_index={d}", .{this.execution.group_index}), .done => try writer.print("done", .{}), } @@ -299,11 +298,18 @@ pub const BunTest = struct { const active_sequence_index = 0; const sequence = &sequences[active_sequence_index]; + const active_entry = sequence.active_entry orelse break :blk .{ + .execution = .{ + .group_index = this.execution.group_index, + .entry_data = null, // the sequence is completed. + }, + }; + break :blk .{ .execution = .{ .group_index = this.execution.group_index, .entry_data = .{ .sequence_index = active_sequence_index, - .entry_index = sequence.active_index, + .entry = active_entry, .remaining_repeat_count = sequence.remaining_repeat_count, }, } }; @@ -817,8 +823,8 @@ pub const DescribeScope = struct { try this.entries.append(.{ .describe = child }); return child; } - pub fn appendTest(this: *DescribeScope, gpa: std.mem.Allocator, name_not_owned: ?[]const u8, callback: ?jsc.JSValue, cfg: ExecutionEntryCfg, base: BaseScopeCfg) bun.JSError!*ExecutionEntry { - const entry = try ExecutionEntry.create(gpa, name_not_owned, callback, cfg, this, base); + pub fn appendTest(this: *DescribeScope, gpa: std.mem.Allocator, name_not_owned: ?[]const u8, callback: ?jsc.JSValue, cfg: ExecutionEntryCfg, base: BaseScopeCfg, phase: ExecutionEntry.AddedInPhase) bun.JSError!*ExecutionEntry { + const entry = try ExecutionEntry.create(gpa, name_not_owned, callback, cfg, this, base, phase); entry.base.propagate(entry.callback != null); try this.entries.append(.{ .test_callback = entry }); return entry; @@ -832,8 +838,8 @@ pub const DescribeScope = struct { .afterAll => return &this.afterAll, } } - pub fn appendHook(this: *DescribeScope, gpa: std.mem.Allocator, tag: HookTag, callback: ?jsc.JSValue, cfg: ExecutionEntryCfg, base: BaseScopeCfg) bun.JSError!*ExecutionEntry { - const entry = try ExecutionEntry.create(gpa, null, callback, cfg, this, base); + pub fn appendHook(this: *DescribeScope, gpa: std.mem.Allocator, tag: HookTag, callback: ?jsc.JSValue, cfg: ExecutionEntryCfg, base: BaseScopeCfg, phase: ExecutionEntry.AddedInPhase) bun.JSError!*ExecutionEntry { + const entry = try ExecutionEntry.create(gpa, null, callback, cfg, this, base, phase); try this.getHookEntries(tag).append(entry); return entry; } @@ -852,13 +858,20 @@ pub const ExecutionEntry = struct { /// '.epoch' = not set /// when this entry begins executing, the timespec will be set to the current time plus the timeout(ms). timespec: bun.timespec = .epoch, + added_in_phase: AddedInPhase, + + next: ?*ExecutionEntry = null, + skip_to: ?*ExecutionEntry = null, + + const AddedInPhase = enum { preload, collection, execution }; - fn create(gpa: std.mem.Allocator, name_not_owned: ?[]const u8, cb: ?jsc.JSValue, cfg: ExecutionEntryCfg, parent: ?*DescribeScope, base: BaseScopeCfg) bun.JSError!*ExecutionEntry { + fn create(gpa: std.mem.Allocator, name_not_owned: ?[]const u8, cb: ?jsc.JSValue, cfg: ExecutionEntryCfg, parent: ?*DescribeScope, base: BaseScopeCfg, phase: AddedInPhase) bun.JSError!*ExecutionEntry { const entry = bun.create(gpa, ExecutionEntry, .{ .base = .init(base, gpa, name_not_owned, parent, cb != null), .callback = null, .timeout = cfg.timeout, .has_done_parameter = cfg.has_done_parameter, + .added_in_phase = phase, }); if (cb) |c| { diff --git a/src/bun.js/test/debug.zig b/src/bun.js/test/debug.zig index 112b8a91165..3b56f5fda9f 100644 --- a/src/bun.js/test/debug.zig +++ b/src/bun.js/test/debug.zig @@ -34,8 +34,9 @@ pub fn dumpOrder(this: *Execution) bun.JSError!void { group.beginMsg("{d} Sequence ({d}x)", .{ sequence_index, sequence.remaining_repeat_count }); defer group.end(); - for (sequence.entries(this), 0..) |entry, entry_index| { - group.log("{d} ExecutionEntry \"{}\" (concurrent={}, mode={s}, only={s}, has_callback={})", .{ entry_index, std.zig.fmtEscapes(entry.base.name orelse "(unnamed)"), entry.base.concurrent, @tagName(entry.base.mode), @tagName(entry.base.only), entry.base.has_callback }); + var current_entry = sequence.first_entry; + while (current_entry) |entry| : (current_entry = entry.next) { + group.log("ExecutionEntry \"{}\" (concurrent={}, mode={s}, only={s}, has_callback={})", .{ std.zig.fmtEscapes(entry.base.name orelse "(unnamed)"), entry.base.concurrent, @tagName(entry.base.mode), @tagName(entry.base.only), entry.base.has_callback }); } } } From 71e0f5437532ecbe1224e885784e7a0eab529529 Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 29 Sep 2025 14:54:12 -0700 Subject: [PATCH 02/11] this works but is not ideal --- src/bun.js/test/Order.zig | 12 +++++++++--- src/bun.js/test/bun_test.zig | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/bun.js/test/Order.zig b/src/bun.js/test/Order.zig index 0ff069aa6b6..84c18df7f53 100644 --- a/src/bun.js/test/Order.zig +++ b/src/bun.js/test/Order.zig @@ -2,12 +2,14 @@ groups: std.ArrayList(ConcurrentGroup), sequences: std.ArrayList(ExecutionSequence), +arena: std.mem.Allocator, previous_group_was_concurrent: bool = false, -pub fn init(gpa: std.mem.Allocator) Order { +pub fn init(gpa: std.mem.Allocator, arena: std.mem.Allocator) Order { return .{ .groups = std.ArrayList(ConcurrentGroup).init(gpa), .sequences = std.ArrayList(ExecutionSequence).init(gpa), + .arena = arena, }; } pub fn deinit(this: *Order) void { @@ -40,6 +42,7 @@ pub const Config = struct { pub fn generateAllOrder(this: *Order, entries: []const *ExecutionEntry, _: Config) bun.JSError!AllOrderResult { const start = this.groups.items.len; for (entries) |entry| { + if (bun.Environment.ci_assert and entry.added_in_phase != .preload) bun.assert(entry.next == null); entry.next = null; entry.skip_to = null; const sequences_start = this.sequences.items.len; @@ -83,7 +86,10 @@ const EntryList = struct { first: ?*ExecutionEntry = null, last: ?*ExecutionEntry = null, pub fn append(this: *EntryList, current: *ExecutionEntry) void { + if (bun.Environment.ci_assert and current.added_in_phase != .preload) bun.assert(current.next == null); + current.next = null; if (this.last) |last| { + if (bun.Environment.ci_assert and last.added_in_phase != .preload) bun.assert(last.next == null); last.next = current; this.last = current; } else { @@ -103,7 +109,7 @@ pub fn generateOrderTest(this: *Order, current: *ExecutionEntry, _: Config) bun. var parent: ?*DescribeScope = current.base.parent; while (parent) |p| : (parent = p.base.parent) { for (p.beforeEach.items) |entry| { - list.append(entry); + list.append(bun.create(this.arena, ExecutionEntry, entry.*)); } } } @@ -116,7 +122,7 @@ pub fn generateOrderTest(this: *Order, current: *ExecutionEntry, _: Config) bun. var parent: ?*DescribeScope = current.base.parent; while (parent) |p| : (parent = p.base.parent) { for (p.afterEach.items) |entry| { - list.append(entry); + list.append(bun.create(this.arena, ExecutionEntry, entry.*)); } } } diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index 4aa28f01b5c..b1b86b05834 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -516,7 +516,7 @@ pub const BunTest = struct { .collection => { this.phase = .execution; try debug.dumpDescribe(this.collection.root_scope); - var order = Order.init(this.gpa); + var order = Order.init(this.gpa, this.arena); defer order.deinit(); const has_filter = if (this.reporter) |reporter| if (reporter.jest.filter_regex) |_| true else false else false; From 5cd152582e2344dee51fb6ea5c398460bfa50a50 Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 29 Sep 2025 15:33:31 -0700 Subject: [PATCH 03/11] afterEach/afterAll inside tests --- src/bun.js/test/Execution.zig | 4 +- src/bun.js/test/bun_test.zig | 60 +++++++++++++++++++++------- test/js/bun/test/bun_test.fixture.ts | 34 ++++++++++++++++ test/js/bun/test/bun_test.test.ts | 20 ++++++++-- 4 files changed, 97 insertions(+), 21 deletions(-) diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index a44249965ec..08f43ab0599 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -408,7 +408,7 @@ pub fn activeGroup(this: *Execution) ?*ConcurrentGroup { if (this.group_index >= this.groups.len) return null; return &this.groups[this.group_index]; } -fn getCurrentAndValidExecutionSequence(this: *Execution, data: bun_test.BunTest.RefDataValue) ?struct { *ExecutionSequence, *ConcurrentGroup } { +pub fn getCurrentAndValidExecutionSequence(this: *Execution, data: bun_test.BunTest.RefDataValue) ?struct { *ExecutionSequence, *ConcurrentGroup } { groupLog.begin(@src()); defer groupLog.end(); @@ -438,7 +438,7 @@ fn getCurrentAndValidExecutionSequence(this: *Execution, data: bun_test.BunTest. groupLog.log("runOneCompleted: the data is for a previous repeat count (outdated)", .{}); return null; } - if (sequence.active_entry != data.execution.entry_data.?.entry) { + if (@as(?*anyopaque, sequence.active_entry) != data.execution.entry_data.?.entry) { groupLog.log("runOneCompleted: the data is for a different sequence index (outdated)", .{}); return null; } diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index b1b86b05834..9167ac5bfed 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -51,26 +51,48 @@ pub const js_fns = struct { const bunTestRoot = try getActiveTestRoot(globalThis, .{ .signature = .{ .str = @tagName(tag) ++ "()" }, .allow_in_preload = true }); + const cfg: ExecutionEntryCfg = .{ + .has_done_parameter = has_done_parameter, + .timeout = args.options.timeout, + }; const bunTest = bunTestRoot.getActiveFileUnlessInPreload(globalThis.bunVM()) orelse { group.log("genericHook in preload", .{}); - _ = try bunTestRoot.hook_scope.appendHook(bunTestRoot.gpa, tag, args.callback, .{ - .has_done_parameter = has_done_parameter, - .timeout = args.options.timeout, - }, .{}, .preload); + _ = try bunTestRoot.hook_scope.appendHook(bunTestRoot.gpa, tag, args.callback, cfg, .{}, .preload); return .js_undefined; }; switch (bunTest.phase) { .collection => { - _ = try bunTest.collection.active_scope.appendHook(bunTest.gpa, tag, args.callback, .{ - .has_done_parameter = has_done_parameter, - .timeout = args.options.timeout, - }, .{}, .collection); + _ = try bunTest.collection.active_scope.appendHook(bunTest.gpa, tag, args.callback, cfg, .{}, .collection); return .js_undefined; }, .execution => { + if (tag == .afterAll or tag == .afterEach) { + // allowed + const active = bunTest.getCurrentStateData(); + const sequence, _ = bunTest.execution.getCurrentAndValidExecutionSequence(active) orelse { + return globalThis.throw("Cannot call {s}() here. It cannot be called inside a concurrent test. Call it inside describe() instead.", .{@tagName(tag)}); + }; + var append_point = sequence.active_entry; + + var iter = append_point; + const before_test_entry = while (iter) |entry| : (iter = entry.next) { + if (entry == sequence.test_entry) break true; + } else false; + + if (before_test_entry) append_point = sequence.test_entry; + + const append_point_value = append_point orelse return globalThis.throw("Cannot call {s}() here. Call it inside describe() instead.", .{@tagName(tag)}); + + const new_item = ExecutionEntry.create(bunTest.gpa, null, args.callback, cfg, null, .{}, .execution); + new_item.next = append_point_value.next; + append_point_value.next = new_item; + bun.handleOom(bunTest.extra_execution_entries.append(new_item)); + + return .js_undefined; + } return globalThis.throw("Cannot call {s}() inside a test. Call it inside describe() instead.", .{@tagName(tag)}); }, .done => return globalThis.throw("Cannot call {s}() after the test run has completed", .{@tagName(tag)}), @@ -158,6 +180,7 @@ pub const BunTest = struct { result_queue: ResultQueue, /// Whether tests in this file should default to concurrent execution default_concurrent: bool, + extra_execution_entries: std.ArrayList(*ExecutionEntry), phase: enum { collection, @@ -190,6 +213,7 @@ pub const BunTest = struct { .reporter = reporter, .result_queue = .init(this.gpa), .default_concurrent = default_concurrent, + .extra_execution_entries = .init(this.gpa), }; } pub fn deinit(this: *BunTest) void { @@ -201,6 +225,11 @@ pub const BunTest = struct { bun.jsc.VirtualMachine.get().timer.remove(&this.timer); } + for (this.extra_execution_entries.items) |entry| { + entry.destroy(this.gpa); + } + this.extra_execution_entries.deinit(); + this.execution.deinit(); this.collection.deinit(); this.result_queue.deinit(); @@ -217,7 +246,7 @@ pub const BunTest = struct { group_index: usize, entry_data: ?struct { sequence_index: usize, - entry: *ExecutionEntry, + entry: *const anyopaque, remaining_repeat_count: i64, }, }, @@ -233,10 +262,11 @@ pub const BunTest = struct { const entry_data = this.execution.entry_data orelse return null; return &group_item.sequences(&buntest.execution)[entry_data.sequence_index]; } - pub fn entry(this: *const RefDataValue, _: *BunTest) ?*ExecutionEntry { + pub fn entry(this: *const RefDataValue, buntest: *BunTest) ?*ExecutionEntry { if (this.* != .execution) return null; - const entry_data = this.execution.entry_data orelse return null; - return entry_data.entry; + if (buntest.phase != .execution) return null; + const the_sequence, _ = buntest.execution.getCurrentAndValidExecutionSequence(this.*) orelse return null; + return the_sequence.active_entry; } pub fn format(this: *const RefDataValue, comptime _: []const u8, _: std.fmt.FormatOptions, writer: anytype) !void { @@ -824,7 +854,7 @@ pub const DescribeScope = struct { return child; } pub fn appendTest(this: *DescribeScope, gpa: std.mem.Allocator, name_not_owned: ?[]const u8, callback: ?jsc.JSValue, cfg: ExecutionEntryCfg, base: BaseScopeCfg, phase: ExecutionEntry.AddedInPhase) bun.JSError!*ExecutionEntry { - const entry = try ExecutionEntry.create(gpa, name_not_owned, callback, cfg, this, base, phase); + const entry = ExecutionEntry.create(gpa, name_not_owned, callback, cfg, this, base, phase); entry.base.propagate(entry.callback != null); try this.entries.append(.{ .test_callback = entry }); return entry; @@ -839,7 +869,7 @@ pub const DescribeScope = struct { } } pub fn appendHook(this: *DescribeScope, gpa: std.mem.Allocator, tag: HookTag, callback: ?jsc.JSValue, cfg: ExecutionEntryCfg, base: BaseScopeCfg, phase: ExecutionEntry.AddedInPhase) bun.JSError!*ExecutionEntry { - const entry = try ExecutionEntry.create(gpa, null, callback, cfg, this, base, phase); + const entry = ExecutionEntry.create(gpa, null, callback, cfg, this, base, phase); try this.getHookEntries(tag).append(entry); return entry; } @@ -865,7 +895,7 @@ pub const ExecutionEntry = struct { const AddedInPhase = enum { preload, collection, execution }; - fn create(gpa: std.mem.Allocator, name_not_owned: ?[]const u8, cb: ?jsc.JSValue, cfg: ExecutionEntryCfg, parent: ?*DescribeScope, base: BaseScopeCfg, phase: AddedInPhase) bun.JSError!*ExecutionEntry { + fn create(gpa: std.mem.Allocator, name_not_owned: ?[]const u8, cb: ?jsc.JSValue, cfg: ExecutionEntryCfg, parent: ?*DescribeScope, base: BaseScopeCfg, phase: AddedInPhase) *ExecutionEntry { const entry = bun.create(gpa, ExecutionEntry, .{ .base = .init(base, gpa, name_not_owned, parent, cb != null), .callback = null, diff --git a/test/js/bun/test/bun_test.fixture.ts b/test/js/bun/test/bun_test.fixture.ts index 29aec332ee6..413cfddf686 100644 --- a/test/js/bun/test/bun_test.fixture.ts +++ b/test/js/bun/test/bun_test.fixture.ts @@ -269,4 +269,38 @@ test.failing("microtasks and rejections are drained after the test callback is e Promise.reject(new Error("uh oh!")); }); +describe("after inside test", () => { + afterAll(() => { + console.log("after-inside-test: afterAll3"); + }); + afterEach(() => { + console.log("after-inside-test: afterEach3"); + }); + + test("the test 1", () => { + afterEach(() => { + console.log("after-inside-test: afterEach1"); + }); + afterAll(() => { + console.log("after-inside-test: afterAll1"); + }); + console.log("after-inside-test: the test 1"); + }); + test("the test 2", () => { + afterEach(() => { + console.log("after-inside-test: afterEach2"); + }); + afterAll(() => { + console.log("after-inside-test: afterAll2"); + }); + console.log("after-inside-test: the test 2"); + }); +}); + +test("beforeAll inside test fails", () => { + expect(() => beforeEach(() => {})).toThrowErrorMatchingInlineSnapshot( + `"Cannot call beforeEach() inside a test. Call it inside describe() instead."`, + ); +}); + console.log("exit"); diff --git a/test/js/bun/test/bun_test.test.ts b/test/js/bun/test/bun_test.test.ts index f57248d3f59..c5d6462a70c 100644 --- a/test/js/bun/test/bun_test.test.ts +++ b/test/js/bun/test/bun_test.test.ts @@ -101,6 +101,9 @@ test("describe/test", async () => { (fail) done parameter > done combined with promise error conditions > promise errors only (pass) done parameter > second call of done callback ignores triggers error (pass) microtasks and rejections are drained after the test callback is executed + (pass) after inside test > the test 1 + (pass) after inside test > the test 2 + (pass) beforeAll inside test fails 2 tests skipped: (skip) LINE 67 @@ -128,13 +131,13 @@ test("describe/test", async () => { (fail) done parameter > done combined with promise error conditions > done errors only (fail) done parameter > done combined with promise error conditions > promise errors only - 29 pass + 32 pass 2 skip 2 todo 10 fail 1 error - 1 snapshots, 9 expect() calls - Ran 43 tests across 1 file." + 2 snapshots, 10 expect() calls + Ran 46 tests across 1 file." , "stdout": "bun test () @@ -199,7 +202,16 @@ test("describe/test", async () => { afterEach1 afterEach2 afterAll1 - afterAll2" + afterAll2 + after-inside-test: the test 1 + after-inside-test: afterAll1 + after-inside-test: afterEach1 + after-inside-test: afterEach3 + after-inside-test: the test 2 + after-inside-test: afterAll2 + after-inside-test: afterEach2 + after-inside-test: afterEach3 + after-inside-test: afterAll3" , } `); From f6587e9820e570d7207cf869b7e471674d922591 Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 29 Sep 2025 15:36:15 -0700 Subject: [PATCH 04/11] reverts test.ts workaround for no beforeAll/Each inside test.ts --- src/js/node/test.ts | 121 +++++++++----------------------------------- 1 file changed, 23 insertions(+), 98 deletions(-) diff --git a/src/js/node/test.ts b/src/js/node/test.ts index d01451babc8..02ebed74c38 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -7,7 +7,6 @@ const { kEmptyObject, throwNotImplemented } = require("internal/shared"); const kDefaultName = ""; const kDefaultFunction = () => {}; const kDefaultOptions = kEmptyObject; -const kDefaultFilePath = undefined; function run() { throwNotImplemented("run()", 5090, "Use `bun:test` in the interim."); @@ -38,13 +37,6 @@ delete assert.CallTracker; delete assert.strict; let checkNotInsideTest: (ctx: TestContext | undefined, fn: string) => void; -let getTestContextHooks: (ctx: TestContext) => { - beforeHooks: Array<() => unknown | Promise>; - afterHooks: Array<() => unknown | Promise>; - beforeEachHooks: Array<() => unknown | Promise>; - afterEachHooks: Array<() => unknown | Promise>; - runHooks: (hooks: Array<() => unknown | Promise>) => Promise; -}; /** * @link https://nodejs.org/api/test.html#class-testcontext @@ -55,10 +47,6 @@ class TestContext { #filePath: string | undefined; #parent?: TestContext; #abortController?: AbortController; - #afterHooks: Array<() => unknown | Promise> = []; - #beforeHooks: Array<() => unknown | Promise> = []; - #beforeEachHooks: Array<() => unknown | Promise> = []; - #afterEachHooks: Array<() => unknown | Promise> = []; constructor( insideTest: boolean, @@ -127,47 +115,27 @@ class TestContext { } before(arg0: unknown, arg1: unknown) { - const { fn, fnInsideTest } = createHook(arg0, arg1); - if (this.#insideTest) { - // When called inside a test, store the hook to run at the appropriate time - this.#beforeHooks.push(fnInsideTest); - } else { - const { beforeAll } = bunTest(); - beforeAll(fn); - } + const { fn } = createHook(arg0, arg1); + const { beforeAll } = bunTest(); + beforeAll(fn); } after(arg0: unknown, arg1: unknown) { - const { fn, fnInsideTest } = createHook(arg0, arg1); - if (this.#insideTest) { - // When called inside a test, store the hook to run at the end of the test - this.#afterHooks.push(fnInsideTest); - } else { - const { afterAll } = bunTest(); - afterAll(fn); - } + const { fn } = createHook(arg0, arg1); + const { afterAll } = bunTest(); + afterAll(fn); } beforeEach(arg0: unknown, arg1: unknown) { - const { fn, fnInsideTest } = createHook(arg0, arg1); - if (this.#insideTest) { - // When called inside a test, store the hook to run for each subtest - this.#beforeEachHooks.push(fnInsideTest); - } else { - const { beforeEach } = bunTest(); - beforeEach(fn); - } + const { fn } = createHook(arg0, arg1); + const { beforeEach } = bunTest(); + beforeEach(fn); } afterEach(arg0: unknown, arg1: unknown) { - const { fn, fnInsideTest } = createHook(arg0, arg1); - if (this.#insideTest) { - // When called inside a test, store the hook to run after each subtest - this.#afterEachHooks.push(fnInsideTest); - } else { - const { afterEach } = bunTest(); - afterEach(fn); - } + const { fn } = createHook(arg0, arg1); + const { afterEach } = bunTest(); + afterEach(fn); } waitFor(_condition: unknown, _options: { timeout?: number } = kEmptyObject) { @@ -200,15 +168,6 @@ class TestContext { describe(name, fn); } - async #runHooks(hooks: Array<() => unknown | Promise>) { - for (const hook of hooks) { - const result = hook(); - if (result instanceof Promise) { - await result; - } - } - } - #checkNotInsideTest(fn: string) { if (this.#insideTest) { throwNotImplemented(`${fn}() inside another test()`, 5090, "Use `bun:test` in the interim."); @@ -216,20 +175,10 @@ class TestContext { } static { - // expose these functions to the rest of this file without exposing them to user JS + // expose this function to the rest of this file without exposing it to user JS checkNotInsideTest = (ctx: TestContext | undefined, fn: string) => { if (ctx) ctx.#checkNotInsideTest(fn); }; - - getTestContextHooks = (ctx: TestContext) => { - return { - beforeHooks: ctx.#beforeHooks, - afterHooks: ctx.#afterHooks, - beforeEachHooks: ctx.#beforeEachHooks, - afterEachHooks: ctx.#afterEachHooks, - runHooks: (hooks: Array<() => unknown | Promise>) => ctx.#runHooks(hooks), - }; - }; } } @@ -353,24 +302,13 @@ function createTest(arg0: unknown, arg1: unknown, arg2: unknown) { const { name, options, fn } = parseTestOptions(arg0, arg1, arg2); checkNotInsideTest(ctx, "test"); - const context = new TestContext(true, name, Bun.main, ctx); + const originalContext = ctx; + const context = new TestContext(true, name, Bun.main, originalContext); - const runTest = async (done: (error?: unknown) => void) => { - const originalContext = ctx; + const runTest = (done: (error?: unknown) => void) => { ctx = context; - const hooks = getTestContextHooks(context); - - const endTest = async (error?: unknown) => { + const endTest = (error?: unknown) => { try { - // Run after hooks before ending the test - if (!error && hooks.afterHooks.length > 0) { - try { - await hooks.runHooks(hooks.afterHooks); - } catch (hookError) { - done(hookError); - return; - } - } done(error); } finally { ctx = originalContext; @@ -379,19 +317,15 @@ function createTest(arg0: unknown, arg1: unknown, arg2: unknown) { let result: unknown; try { - // Run before hooks before running the test - if (hooks.beforeHooks.length > 0) { - await hooks.runHooks(hooks.beforeHooks); - } result = fn(context); } catch (error) { - await endTest(error); + endTest(error); return; } if (result instanceof Promise) { (result as Promise).then(() => endTest()).catch(error => endTest(error)); } else { - await endTest(); + endTest(); } }; @@ -402,10 +336,10 @@ function createDescribe(arg0: unknown, arg1: unknown, arg2: unknown) { const { name, fn, options } = parseTestOptions(arg0, arg1, arg2); checkNotInsideTest(ctx, "describe"); - const context = new TestContext(false, name, Bun.main, ctx); + const originalContext = ctx; + const context = new TestContext(false, name, Bun.main, originalContext); const runDescribe = () => { - const originalContext = ctx; ctx = context; const endDescribe = () => { ctx = originalContext; @@ -443,16 +377,7 @@ function parseHookOptions(arg0: unknown, arg1: unknown) { function createHook(arg0: unknown, arg1: unknown) { const { fn, options } = parseHookOptions(arg0, arg1); - // When used inside a test context, we don't have done callback - const runHookInsideTest = async () => { - const result = fn(); - if (result instanceof Promise) { - await result; - } - }; - - // When used at module level, we have done callback - const runHookWithDone = (done: (error?: unknown) => void) => { + const runHook = (done: (error?: unknown) => void) => { let result: unknown; try { result = fn(); @@ -467,7 +392,7 @@ function createHook(arg0: unknown, arg1: unknown) { } }; - return { options, fn: runHookWithDone, fnInsideTest: runHookInsideTest }; + return { options, fn: runHook }; } type TestFn = (ctx: TestContext) => unknown | Promise; From 10d8c19787124137b96894486b171caad3ba0cb3 Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 29 Sep 2025 18:00:25 -0700 Subject: [PATCH 05/11] 23077 repro --- test/regression/issue/23077/23077.test.ts | 14 ++++++++++++++ test/regression/issue/23077/a.fixture.ts | 2 ++ test/regression/issue/23077/b.fixture.ts | 2 ++ 3 files changed, 18 insertions(+) create mode 100644 test/regression/issue/23077/23077.test.ts create mode 100644 test/regression/issue/23077/a.fixture.ts create mode 100644 test/regression/issue/23077/b.fixture.ts diff --git a/test/regression/issue/23077/23077.test.ts b/test/regression/issue/23077/23077.test.ts new file mode 100644 index 00000000000..7562aa84c92 --- /dev/null +++ b/test/regression/issue/23077/23077.test.ts @@ -0,0 +1,14 @@ +import { test, expect } from "bun:test"; +import { bunExe } from "harness"; + +test("23077", async () => { + const result = Bun.spawn({ + cmd: [bunExe(), "test", import.meta.dir + "/a.fixture.ts", import.meta.dir + "/b.fixture.ts"], + stdio: ["pipe", "pipe", "pipe"], + }); + const exitCode = await result.exited; + const stdout = await result.stdout.text(); + const stderr = await result.stderr.text(); + expect(stderr).toInclude(" 2 pass"); + expect(exitCode).toBe(0); +}); diff --git a/test/regression/issue/23077/a.fixture.ts b/test/regression/issue/23077/a.fixture.ts new file mode 100644 index 00000000000..d27806ad2d0 --- /dev/null +++ b/test/regression/issue/23077/a.fixture.ts @@ -0,0 +1,2 @@ +import { test } from "node:test"; +test("one", async () => await Bun.sleep(5)); diff --git a/test/regression/issue/23077/b.fixture.ts b/test/regression/issue/23077/b.fixture.ts new file mode 100644 index 00000000000..d27806ad2d0 --- /dev/null +++ b/test/regression/issue/23077/b.fixture.ts @@ -0,0 +1,2 @@ +import { test } from "node:test"; +test("one", async () => await Bun.sleep(5)); From c312da54f4fb91e2668de9b0a38599dc8dafe887 Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 29 Sep 2025 18:24:59 -0700 Subject: [PATCH 06/11] fixes the beforeEach order --- repro.ts | 24 +++++++++++++++++++ src/bun.js/test/Order.zig | 14 ++++++++--- .../third_party/grpc-js/test-metadata.test.ts | 1 + 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 repro.ts diff --git a/repro.ts b/repro.ts new file mode 100644 index 00000000000..74722c68bdf --- /dev/null +++ b/repro.ts @@ -0,0 +1,24 @@ +import { expect, it, afterEach, test, describe, beforeEach, beforeAll } from "bun:test"; + +let set: Set | null; +beforeEach(() => { + console.log("outer beforeEach"); + set = new Set(); +}); +afterEach(() => { + console.log("outer afterEach"); + set = null; +}); + +describe("get", () => { + beforeEach(() => { + console.log("inner beforeEach"); + set!.add("value1"); + set!.add("value2"); + }); + + it("gets all values associated with a key", () => { + console.log("inner it"); + expect(set!.size).toBe(2); + }); +}); diff --git a/src/bun.js/test/Order.zig b/src/bun.js/test/Order.zig index 84c18df7f53..de48849fa86 100644 --- a/src/bun.js/test/Order.zig +++ b/src/bun.js/test/Order.zig @@ -85,6 +85,11 @@ pub fn generateOrderDescribe(this: *Order, current: *DescribeScope, cfg: Config) const EntryList = struct { first: ?*ExecutionEntry = null, last: ?*ExecutionEntry = null, + pub fn prepend(this: *EntryList, current: *ExecutionEntry) void { + current.next = this.first; + this.first = current; + if (this.last == null) this.last = current; + } pub fn append(this: *EntryList, current: *ExecutionEntry) void { if (bun.Environment.ci_assert and current.added_in_phase != .preload) bun.assert(current.next == null); current.next = null; @@ -98,6 +103,7 @@ const EntryList = struct { } } }; + pub fn generateOrderTest(this: *Order, current: *ExecutionEntry, _: Config) bun.JSError!void { bun.assert(current.base.has_callback == (current.callback != null)); const use_each_hooks = current.base.has_callback; @@ -108,8 +114,10 @@ pub fn generateOrderTest(this: *Order, current: *ExecutionEntry, _: Config) bun. if (use_each_hooks) { var parent: ?*DescribeScope = current.base.parent; while (parent) |p| : (parent = p.base.parent) { - for (p.beforeEach.items) |entry| { - list.append(bun.create(this.arena, ExecutionEntry, entry.*)); + // prepend in reverse so they end up in forwards order + var i: usize = p.beforeEach.items.len; + while (i > 0) : (i -= 1) { + list.prepend(bun.create(this.arena, ExecutionEntry, p.beforeEach.items[i - 1].*)); } } } @@ -132,7 +140,7 @@ pub fn generateOrderTest(this: *Order, current: *ExecutionEntry, _: Config) bun. var skip_to = current.next; while (index) |entry| : (index = entry.next) { if (entry == skip_to) skip_to = null; - entry.skip_to = skip_to; + entry.skip_to = skip_to; // we should consider matching skip_to in beforeAll to skip directly to the first afterAll from its own scope rather than skipping to the first afterAll from any scope } // add these as a single sequence diff --git a/test/js/third_party/grpc-js/test-metadata.test.ts b/test/js/third_party/grpc-js/test-metadata.test.ts index 54310bc4d60..3b6463358b6 100644 --- a/test/js/third_party/grpc-js/test-metadata.test.ts +++ b/test/js/third_party/grpc-js/test-metadata.test.ts @@ -42,6 +42,7 @@ describe("Metadata", () => { let metadata: TestMetadata; beforeEach(() => { + console.log("create testmetadata"); metadata = new TestMetadata(); }); From 4ba5f921debdf74ef0ad430c311d590fbc0a41ad Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 29 Sep 2025 18:30:24 -0700 Subject: [PATCH 07/11] different version of test.ts --- src/js/node/test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 02ebed74c38..a2d51e69a50 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -302,10 +302,10 @@ function createTest(arg0: unknown, arg1: unknown, arg2: unknown) { const { name, options, fn } = parseTestOptions(arg0, arg1, arg2); checkNotInsideTest(ctx, "test"); - const originalContext = ctx; - const context = new TestContext(true, name, Bun.main, originalContext); + const context = new TestContext(true, name, Bun.main, ctx); const runTest = (done: (error?: unknown) => void) => { + const originalContext = ctx; ctx = context; const endTest = (error?: unknown) => { try { @@ -336,10 +336,10 @@ function createDescribe(arg0: unknown, arg1: unknown, arg2: unknown) { const { name, fn, options } = parseTestOptions(arg0, arg1, arg2); checkNotInsideTest(ctx, "describe"); - const originalContext = ctx; - const context = new TestContext(false, name, Bun.main, originalContext); + const context = new TestContext(false, name, Bun.main, ctx); const runDescribe = () => { + const originalContext = ctx; ctx = context; const endDescribe = () => { ctx = originalContext; From 4c3884469dc8a4512da09b2202ef86af5c143deb Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 30 Sep 2025 01:34:07 +0000 Subject: [PATCH 08/11] [autofix.ci] apply automated fixes --- test/regression/issue/23077/23077.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/regression/issue/23077/23077.test.ts b/test/regression/issue/23077/23077.test.ts index 7562aa84c92..c9dc56de099 100644 --- a/test/regression/issue/23077/23077.test.ts +++ b/test/regression/issue/23077/23077.test.ts @@ -1,4 +1,4 @@ -import { test, expect } from "bun:test"; +import { expect, test } from "bun:test"; import { bunExe } from "harness"; test("23077", async () => { From 37d8b9d9e94bc38c9acfce67cdd6f3e0bd4f85b2 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Mon, 29 Sep 2025 20:55:47 -0700 Subject: [PATCH 09/11] Delete repro.ts --- repro.ts | 24 ------------------------ 1 file changed, 24 deletions(-) delete mode 100644 repro.ts diff --git a/repro.ts b/repro.ts deleted file mode 100644 index 74722c68bdf..00000000000 --- a/repro.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { expect, it, afterEach, test, describe, beforeEach, beforeAll } from "bun:test"; - -let set: Set | null; -beforeEach(() => { - console.log("outer beforeEach"); - set = new Set(); -}); -afterEach(() => { - console.log("outer afterEach"); - set = null; -}); - -describe("get", () => { - beforeEach(() => { - console.log("inner beforeEach"); - set!.add("value1"); - set!.add("value2"); - }); - - it("gets all values associated with a key", () => { - console.log("inner it"); - expect(set!.size).toBe(2); - }); -}); From 5d8b74c74990efad72c0ce770da09a29573ce435 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Mon, 29 Sep 2025 20:55:56 -0700 Subject: [PATCH 10/11] Delete TODO.md --- TODO.md | 1 - 1 file changed, 1 deletion(-) delete mode 100644 TODO.md diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 07c8381d4cd..00000000000 --- a/TODO.md +++ /dev/null @@ -1 +0,0 @@ -- [ ] revert changes made to node:test implementation From 3a41f13fb6b968522cd2bcbc5ebc7a2b6513a7a8 Mon Sep 17 00:00:00 2001 From: pfg Date: Tue, 30 Sep 2025 17:14:41 -0700 Subject: [PATCH 11/11] cleanup --- test/js/bun/test/bun_test.fixture.ts | 2 +- test/js/bun/test/bun_test.test.ts | 2 +- test/js/third_party/grpc-js/test-metadata.test.ts | 1 - test/regression/issue/23077/23077.test.ts | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/test/js/bun/test/bun_test.fixture.ts b/test/js/bun/test/bun_test.fixture.ts index 413cfddf686..b47b15738b7 100644 --- a/test/js/bun/test/bun_test.fixture.ts +++ b/test/js/bun/test/bun_test.fixture.ts @@ -297,7 +297,7 @@ describe("after inside test", () => { }); }); -test("beforeAll inside test fails", () => { +test("beforeEach inside test fails", () => { expect(() => beforeEach(() => {})).toThrowErrorMatchingInlineSnapshot( `"Cannot call beforeEach() inside a test. Call it inside describe() instead."`, ); diff --git a/test/js/bun/test/bun_test.test.ts b/test/js/bun/test/bun_test.test.ts index c5d6462a70c..147295d9830 100644 --- a/test/js/bun/test/bun_test.test.ts +++ b/test/js/bun/test/bun_test.test.ts @@ -103,7 +103,7 @@ test("describe/test", async () => { (pass) microtasks and rejections are drained after the test callback is executed (pass) after inside test > the test 1 (pass) after inside test > the test 2 - (pass) beforeAll inside test fails + (pass) beforeEach inside test fails 2 tests skipped: (skip) LINE 67 diff --git a/test/js/third_party/grpc-js/test-metadata.test.ts b/test/js/third_party/grpc-js/test-metadata.test.ts index 3b6463358b6..54310bc4d60 100644 --- a/test/js/third_party/grpc-js/test-metadata.test.ts +++ b/test/js/third_party/grpc-js/test-metadata.test.ts @@ -42,7 +42,6 @@ describe("Metadata", () => { let metadata: TestMetadata; beforeEach(() => { - console.log("create testmetadata"); metadata = new TestMetadata(); }); diff --git a/test/regression/issue/23077/23077.test.ts b/test/regression/issue/23077/23077.test.ts index c9dc56de099..e88385d4973 100644 --- a/test/regression/issue/23077/23077.test.ts +++ b/test/regression/issue/23077/23077.test.ts @@ -2,7 +2,7 @@ import { expect, test } from "bun:test"; import { bunExe } from "harness"; test("23077", async () => { - const result = Bun.spawn({ + await using result = Bun.spawn({ cmd: [bunExe(), "test", import.meta.dir + "/a.fixture.ts", import.meta.dir + "/b.fixture.ts"], stdio: ["pipe", "pipe", "pipe"], });