From b2c219a56c3ebcb49f0a47763f88c506d7e38773 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 14 Nov 2025 16:21:04 -0800 Subject: [PATCH] Implement retry and repeats options for bun:test (#23713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #16051, Fixes ENG-21437 Implements retry/repeats ```ts test("my test", () => { if (Math.random() < 0.1) throw new Error("uh oh!"); }, {repeats: 20}); ``` ``` Error: uh oh! ✗ my test ``` ```ts test("my test", () => { if (Math.random() < 0.1) throw new Error("uh oh!"); }, {retry: 5}); ``` ``` Error: uh oh! ✓ my test (attempt 2) ``` Also fixes a bug where onTestFinished inside a test would not run if the test failed ```ts test("abc", () => { onTestFinished(() => { console.log("hello" }); throw new Error("uh oh!"); }); ``` ``` Error: uh oh! hello ``` --------- Co-authored-by: Claude Bot Co-authored-by: pfg Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/test/Execution.zig | 74 +++++--- src/bun.js/test/Order.zig | 20 ++- src/bun.js/test/ScopeFunctions.zig | 28 +-- src/bun.js/test/bun_test.zig | 13 +- src/cli/test_command.zig | 14 ++ .../js/bun/test/test-on-test-finished.test.ts | 16 ++ .../bun/test/test-retry-repeats-basic.test.ts | 161 ++++++++++++++++++ 7 files changed, 283 insertions(+), 43 deletions(-) create mode 100644 test/js/bun/test/test-retry-repeats-basic.test.ts diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index 69f90ac347..77669082cf 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -76,7 +76,8 @@ pub const ExecutionSequence = struct { /// Index into ExecutionSequence.entries() for the entry that is not started or currently running active_entry: ?*ExecutionEntry, test_entry: ?*ExecutionEntry, - remaining_repeat_count: i64 = 1, + remaining_repeat_count: u32, + remaining_retry_count: u32, result: Result = .pending, executing: bool = false, started_at: bun.timespec = .epoch, @@ -90,11 +91,18 @@ pub const ExecutionSequence = struct { } = .not_set, maybe_skip: bool = false, - pub fn init(first_entry: ?*ExecutionEntry, test_entry: ?*ExecutionEntry) ExecutionSequence { + pub fn init(cfg: struct { + first_entry: ?*ExecutionEntry, + test_entry: ?*ExecutionEntry, + retry_count: u32 = 0, + repeat_count: u32 = 0, + }) ExecutionSequence { return .{ - .first_entry = first_entry, - .active_entry = first_entry, - .test_entry = test_entry, + .first_entry = cfg.first_entry, + .active_entry = cfg.first_entry, + .test_entry = cfg.test_entry, + .remaining_repeat_count = cfg.repeat_count, + .remaining_retry_count = cfg.retry_count, }; } @@ -349,8 +357,10 @@ fn stepSequenceOne(buntest_strong: bun_test.BunTestPtr, globalThis: *jsc.JSGloba } 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.", .{}); + // Sequence is complete - either because: + // 1. It ran out of entries (normal completion) + // 2. All retry/repeat attempts have been exhausted + groupLog.log("runOne: no more entries; sequence complete.", .{}); return .done; }; sequence.executing = true; @@ -455,7 +465,7 @@ fn advanceSequence(this: *Execution, sequence: *ExecutionSequence, group: *Concu sequence.executing = false; if (sequence.maybe_skip) { sequence.maybe_skip = false; - sequence.active_entry = entry.skip_to; + sequence.active_entry = if (entry.failure_skip_past) |failure_skip_past| failure_skip_past.next else null; } else { sequence.active_entry = entry.next; } @@ -465,18 +475,32 @@ fn advanceSequence(this: *Execution, sequence: *ExecutionSequence, group: *Concu if (sequence.active_entry == null) { // just completed the sequence - this.onSequenceCompleted(sequence); - sequence.remaining_repeat_count -= 1; - if (sequence.remaining_repeat_count <= 0) { - // no repeats left; indicate completion - if (group.remaining_incomplete_entries == 0) { - bun.debugAssert(false); // remaining_incomplete_entries should never go below 0 - return; - } - group.remaining_incomplete_entries -= 1; - } else { + const test_failed = sequence.result.isFail(); + const test_passed = sequence.result.isPass(.pending_is_pass); + + // Handle retry logic: if test failed and we have retries remaining, retry it + if (test_failed and sequence.remaining_retry_count > 0) { + sequence.remaining_retry_count -= 1; this.resetSequence(sequence); + return; } + + // Handle repeat logic: if test passed and we have repeats remaining, repeat it + if (test_passed and sequence.remaining_repeat_count > 0) { + sequence.remaining_repeat_count -= 1; + this.resetSequence(sequence); + return; + } + + // Only report the final result after all retries/repeats are done + this.onSequenceCompleted(sequence); + + // No more retries or repeats; mark sequence as complete + if (group.remaining_incomplete_entries == 0) { + bun.debugAssert(false); // remaining_incomplete_entries should never go below 0 + return; + } + group.remaining_incomplete_entries -= 1; } } fn onGroupStarted(_: *Execution, _: *ConcurrentGroup, globalThis: *jsc.JSGlobalObject) void { @@ -580,13 +604,13 @@ pub fn resetSequence(this: *Execution, sequence: *ExecutionSequence) void { } } - if (sequence.result.isPass(.pending_is_pass)) { - // passed or pending; run again - sequence.* = .init(sequence.first_entry, sequence.test_entry); - } else { - // already failed or skipped; don't run again - sequence.active_entry = null; - } + // Preserve the current remaining_repeat_count and remaining_retry_count + sequence.* = .init(.{ + .first_entry = sequence.first_entry, + .test_entry = sequence.test_entry, + .retry_count = sequence.remaining_retry_count, + .repeat_count = sequence.remaining_repeat_count, + }); _ = this; } diff --git a/src/bun.js/test/Order.zig b/src/bun.js/test/Order.zig index df2902c766..b1df67422e 100644 --- a/src/bun.js/test/Order.zig +++ b/src/bun.js/test/Order.zig @@ -46,9 +46,12 @@ pub fn generateAllOrder(this: *Order, entries: []const *ExecutionEntry) bun.JSEr 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; + entry.failure_skip_past = null; const sequences_start = this.sequences.items.len; - try this.sequences.append(.init(entry, null)); // add sequence to concurrentgroup + try this.sequences.append(.init(.{ + .first_entry = entry, + .test_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; @@ -139,15 +142,20 @@ pub fn generateOrderTest(this: *Order, current: *ExecutionEntry) bun.JSError!voi // set skip_to values var index = list.first; - var skip_to = current.next; + var failure_skip_past: ?*ExecutionEntry = current; while (index) |entry| : (index = entry.next) { - if (entry == skip_to) skip_to = null; - 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 + entry.failure_skip_past = failure_skip_past; // we could 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 + if (entry == failure_skip_past) failure_skip_past = null; } // add these as a single sequence const sequences_start = this.sequences.items.len; - try this.sequences.append(.init(list.first, current)); // add sequence to concurrentgroup + try this.sequences.append(.init(.{ + .first_entry = list.first, + .test_entry = current, + .retry_count = current.retry_count, + .repeat_count = current.repeat_count, + })); // 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 ba08f8b1c9..9c44d3b388 100644 --- a/src/bun.js/test/ScopeFunctions.zig +++ b/src/bun.js/test/ScopeFunctions.zig @@ -132,10 +132,10 @@ pub fn callAsFunction(globalThis: *JSGlobalObject, callFrame: *CallFrame) bun.JS defer if (formatted_label) |label| bunTest.gpa.free(label); const bound = if (args.callback) |cb| try cb.bind(globalThis, item, &bun.String.static("cb"), 0, args_list_raw.items) else null; - try this.enqueueDescribeOrTestCallback(bunTest, globalThis, callFrame, bound, formatted_label, args.options.timeout, callback_length -| args_list.items.len, line_no); + try this.enqueueDescribeOrTestCallback(bunTest, globalThis, callFrame, bound, formatted_label, args.options, callback_length -| args_list.items.len, line_no); } } else { - try this.enqueueDescribeOrTestCallback(bunTest, globalThis, callFrame, args.callback, args.description, args.options.timeout, callback_length, line_no); + try this.enqueueDescribeOrTestCallback(bunTest, globalThis, callFrame, args.callback, args.description, args.options, callback_length, line_no); } return .js_undefined; @@ -169,7 +169,7 @@ fn filterNames(comptime Rem: type, rem: *Rem, description: ?[]const u8, parent_i } } -fn enqueueDescribeOrTestCallback(this: *ScopeFunctions, bunTest: *bun_test.BunTest, globalThis: *jsc.JSGlobalObject, callFrame: *jsc.CallFrame, callback: ?jsc.JSValue, description: ?[]const u8, timeout: u32, callback_length: usize, line_no: u32) bun.JSError!void { +fn enqueueDescribeOrTestCallback(this: *ScopeFunctions, bunTest: *bun_test.BunTest, globalThis: *jsc.JSGlobalObject, callFrame: *jsc.CallFrame, callback: ?jsc.JSValue, description: ?[]const u8, options: ParseArgumentsOptions, callback_length: usize, line_no: u32) bun.JSError!void { groupLog.begin(@src()); defer groupLog.end(); @@ -248,7 +248,9 @@ 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, + .timeout = options.timeout, + .retry_count = options.retry, + .repeat_count = options.repeats, }, base, .collection); }, } @@ -286,15 +288,16 @@ fn errorInCI(globalThis: *jsc.JSGlobalObject, signature: []const u8) bun.JSError const ParseArgumentsResult = struct { description: ?[]const u8, callback: ?jsc.JSValue, - options: struct { - timeout: u32 = 0, - retry: ?f64 = null, - repeats: ?f64 = null, - }, + options: ParseArgumentsOptions, pub fn deinit(this: *ParseArgumentsResult, gpa: std.mem.Allocator) void { if (this.description) |str| gpa.free(str); } }; +const ParseArgumentsOptions = struct { + timeout: u32 = 0, + retry: u32 = 0, + repeats: u32 = 0, +}; pub const CallbackMode = enum { require, allow }; pub const FunctionKind = enum { test_or_describe, hook }; @@ -382,13 +385,16 @@ pub fn parseArguments(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame if (!retries.isNumber()) { return globalThis.throwPretty("{f}() expects retry to be a number", .{signature}); } - result.options.retry = retries.asNumber(); + result.options.retry = std.math.lossyCast(u32, retries.asNumber()); } if (try options.get(globalThis, "repeats")) |repeats| { if (!repeats.isNumber()) { return globalThis.throwPretty("{f}() expects repeats to be a number", .{signature}); } - result.options.repeats = repeats.asNumber(); + if (result.options.retry != 0) { + return globalThis.throwPretty("{f}(): Cannot set both retry and repeats", .{signature}); + } + result.options.repeats = std.math.lossyCast(u32, repeats.asNumber()); } } else if (options.isUndefinedOrNull()) { // no options diff --git a/src/bun.js/test/bun_test.zig b/src/bun.js/test/bun_test.zig index 6931ca5780..83a0e9b858 100644 --- a/src/bun.js/test/bun_test.zig +++ b/src/bun.js/test/bun_test.zig @@ -924,6 +924,10 @@ pub const ExecutionEntryCfg = struct { /// 0 = unlimited timeout timeout: u32, has_done_parameter: bool, + /// Number of times to retry a failed test (0 = no retries) + retry_count: u32 = 0, + /// Number of times to repeat a test (0 = run once, 1 = run twice, etc.) + repeat_count: u32 = 0, }; pub const ExecutionEntry = struct { base: BaseScope, @@ -935,9 +939,14 @@ pub const ExecutionEntry = struct { /// 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, + /// Number of times to retry a failed test (0 = no retries) + retry_count: u32, + /// Number of times to repeat a test (0 = run once, 1 = run twice, etc.) + repeat_count: u32, next: ?*ExecutionEntry = null, - skip_to: ?*ExecutionEntry = null, + /// if this entry fails, go to the entry 'failure_skip_past.next' + failure_skip_past: ?*ExecutionEntry = null, const AddedInPhase = enum { preload, collection, execution }; @@ -948,6 +957,8 @@ pub const ExecutionEntry = struct { .timeout = cfg.timeout, .has_done_parameter = cfg.has_done_parameter, .added_in_phase = phase, + .retry_count = cfg.retry_count, + .repeat_count = cfg.repeat_count, }); if (cb) |c| { diff --git a/src/cli/test_command.zig b/src/cli/test_command.zig index 015efd3bcf..5464a062ab 100644 --- a/src/cli/test_command.zig +++ b/src/cli/test_command.zig @@ -604,6 +604,10 @@ pub const CommandLineReporter = struct { writer: anytype, comptime dim: bool, ) void { + const initial_retry_count = test_entry.retry_count; + const attempts = (initial_retry_count - sequence.remaining_retry_count) + 1; + const initial_repeat_count = test_entry.repeat_count; + const repeats = (initial_repeat_count - sequence.remaining_repeat_count) + 1; var scopes_stack = bun.BoundedArray(*bun_test.DescribeScope, 64).init(0) catch unreachable; var parent_: ?*bun_test.DescribeScope = test_entry.base.parent; @@ -677,6 +681,16 @@ pub const CommandLineReporter = struct { else writer.print(comptime Output.prettyFmt(" {s}", false), .{display_label}) catch unreachable; + // Print attempt count if test was retried (attempts > 1) + if (attempts > 1) switch (Output.enable_ansi_colors_stderr) { + inline else => |enable_ansi_colors_stderr| writer.print(comptime Output.prettyFmt(" (attempt {d})", enable_ansi_colors_stderr), .{attempts}) catch unreachable, + }; + + // Print repeat count if test failed on a repeat (repeats > 1) + if (repeats > 1) switch (Output.enable_ansi_colors_stderr) { + inline else => |enable_ansi_colors_stderr| writer.print(comptime Output.prettyFmt(" (run {d})", enable_ansi_colors_stderr), .{repeats}) catch unreachable, + }; + if (elapsed_ns > (std.time.ns_per_us * 10)) { writer.print(" {f}", .{ Output.ElapsedFormatter{ diff --git a/test/js/bun/test/test-on-test-finished.test.ts b/test/js/bun/test/test-on-test-finished.test.ts index e97a24a2e4..de75e47347 100644 --- a/test/js/bun/test/test-on-test-finished.test.ts +++ b/test/js/bun/test/test-on-test-finished.test.ts @@ -114,3 +114,19 @@ describe("onTestFinished with all hooks", () => { expect(output).toEqual(["test", "inner afterAll", "afterEach", "onTestFinished"]); }); }); + +// Test that a failing test still runs the onTestFinished hook +describe("onTestFinished with failing test", () => { + const output: string[] = []; + + test.failing("failing test", () => { + onTestFinished(() => { + output.push("onTestFinished"); + }); + output.push("test"); + throw new Error("fail"); + }); + test("verify order", () => { + expect(output).toEqual(["test", "onTestFinished"]); + }); +}); diff --git a/test/js/bun/test/test-retry-repeats-basic.test.ts b/test/js/bun/test/test-retry-repeats-basic.test.ts new file mode 100644 index 0000000000..44e338dbec --- /dev/null +++ b/test/js/bun/test/test-retry-repeats-basic.test.ts @@ -0,0 +1,161 @@ +// Basic tests to verify retry and repeats functionality works +import { afterAll, afterEach, beforeEach, describe, expect, onTestFinished, test } from "bun:test"; + +describe("retry option", () => { + let attempts = 0; + test( + "retries failed test until it passes", + () => { + attempts++; + if (attempts < 3) { + throw new Error("fail"); + } + }, + { retry: 3 }, + ); + test("correct number of attempts from previous test", () => { + expect(attempts).toBe(3); + }); +}); + +describe("repeats option with hooks", () => { + let log: string[] = []; + describe("isolated test with repeats", () => { + beforeEach(() => { + log.push("beforeEach"); + }); + + afterEach(() => { + log.push("afterEach"); + }); + + test( + "repeats test multiple times", + () => { + log.push("test"); + }, + { repeats: 2 }, + ); + }); + + test("verify hooks ran for each repeat", () => { + // Should have: beforeEach, test, afterEach (first), beforeEach, test, afterEach (second), beforeEach, test, afterEach (third) + // repeats: 2 means 1 initial + 2 repeats = 3 total runs + expect(log).toEqual([ + "beforeEach", + "test", + "afterEach", + "beforeEach", + "test", + "afterEach", + "beforeEach", + "test", + "afterEach", + ]); + }); +}); + +describe("retry option with hooks", () => { + let attempts = 0; + let log: string[] = []; + describe("isolated test with retry", () => { + beforeEach(() => { + log.push("beforeEach"); + }); + + afterEach(() => { + log.push("afterEach"); + }); + + test( + "retries with hooks", + () => { + attempts++; + log.push(`test-${attempts}`); + if (attempts < 2) { + throw new Error("fail"); + } + }, + { retry: 3 }, + ); + }); + + test("verify hooks ran for each retry", () => { + // Should have: beforeEach, test-1, afterEach (fail), beforeEach, test-2, afterEach (pass) + expect(log).toEqual(["beforeEach", "test-1", "afterEach", "beforeEach", "test-2", "afterEach"]); + }); +}); +describe("repeats with onTestFinished", () => { + let log: string[] = []; + test( + "repeats with onTestFinished", + () => { + onTestFinished(() => { + log.push("onTestFinished"); + }); + log.push("test"); + }, + { repeats: 3 }, + ); + test("verify correct log", () => { + // repeats: 3 means 1 initial + 3 repeats = 4 total runs + expect(log).toEqual([ + "test", + "onTestFinished", + "test", + "onTestFinished", + "test", + "onTestFinished", + "test", + "onTestFinished", + ]); + }); +}); + +describe("retry with onTestFinished", () => { + let attempts = 0; + let log: string[] = []; + test( + "retry with onTestFinished", + () => { + attempts++; + onTestFinished(() => { + log.push("onTestFinished"); + }); + log.push(`test-${attempts}`); + if (attempts < 3) { + throw new Error("fail"); + } + }, + { retry: 3 }, + ); + test("verify correct log", () => { + expect(log).toEqual(["test-1", "onTestFinished", "test-2", "onTestFinished", "test-3", "onTestFinished"]); + }); +}); + +describe("retry with inner afterAll", () => { + let attempts = 0; + let log: string[] = []; + test( + "retry with inner afterAll", + () => { + attempts++; + afterAll(() => { + log.push("inner afterAll"); + }); + log.push(`test-${attempts}`); + if (attempts < 3) { + throw new Error("fail"); + } + }, + { retry: 3 }, + ); + test("verify correct log", () => { + expect(log).toEqual(["test-1", "inner afterAll", "test-2", "inner afterAll", "test-3", "inner afterAll"]); + }); +}); + +expect(() => { + test("can't pass both", () => {}, { retry: 5, repeats: 6 }); +}).toThrow(/Cannot set both retry and repeats/);