bun test: ignore completions from an earlier attempt of a retried test

src/runtime/test_runner/Execution.rs

@@ -93,7 +93,7 @@ pub struct Execution {
     /// around `run_test_callback` so code re-entered from a test body (e.g.
     /// spawnSync's wait loop) can read the calling entry's own deadline.
     pub(crate) on_stack_entry: core::cell::Cell<Option<NonNull<ExecutionEntry>>>,
-    /// The (group_index, sequence_index, entry, repeat) for `on_stack_entry`,
+    /// The (sequence_index, entry, generation) for `on_stack_entry`,
     /// set/restored alongside it. `get_current_state_data()` can't name a
     /// sequence inside a concurrent group; this can, for code re-entered from
     /// the microtask drain inside `run_test_callback` (node:test's runtime
@@ -159,6 +159,8 @@ pub struct ExecutionSequence {
     pub(crate) test_entry: Option<NonNull<ExecutionEntry>>,
     pub(crate) remaining_repeat_count: u32,
     pub(crate) remaining_retry_count: u32,
+    /// Bumped by every [`Execution::reset_sequence`]; an [`EntryData`] with an older one is from an earlier retry/repeat.
+    pub(crate) generation: u32,
     pub(crate) result: Result,
     pub(crate) executing: bool,
     pub(crate) started_at: Timespec,
@@ -183,6 +185,7 @@ impl ExecutionSequence {
             remaining_repeat_count: repeat_count,
             remaining_retry_count: retry_count,
             // defaults:
+            generation: 0,
             result: Result::Pending,
             executing: false,
             started_at: Timespec::EPOCH,
@@ -475,9 +478,9 @@ impl Execution {
             return None;
         }
         let sequence = &mut self.sequences[seq_abs];
-        if i64::from(sequence.remaining_repeat_count) != entry_data.remaining_repeat_count {
+        if sequence.generation != entry_data.generation {
             group_log::log(format_args!(
-                "runOneCompleted: the data is for a previous repeat count (outdated)",
+                "runOneCompleted: the data is for a previous retry/repeat of the sequence (outdated)",
             ));
             return None;
         }
@@ -740,12 +743,14 @@ impl Execution {
         }
 
         // Preserve retry/repeat counts across reset
+        let generation = sequence.generation;
         *sequence = ExecutionSequence::init(
             sequence.first_entry,
             sequence.test_entry,
             sequence.remaining_retry_count,
             sequence.remaining_repeat_count,
         );
+        sequence.generation = generation.wrapping_add(1);
 
         // Snapshot counters are keyed by full test name and incremented on every
         // toMatchSnapshot() call. Without this reset, retries / repeats would
@@ -1004,7 +1009,7 @@ fn step_sequence_one(
         let entry_data = EntryData {
             sequence_index,
             entry: next_item_ptr.as_ptr() as *const (),
-            remaining_repeat_count: sequence.remaining_repeat_count as i64,
+            generation: sequence.generation,
         };
         let callback_data = RefDataValue::Execution {
             group_index: this.group_index,

src/runtime/test_runner/bun_test.rs

@@ -721,7 +721,7 @@ impl BunTest {
                     entry_data: Some(EntryData {
                         sequence_index: active_sequence_index,
                         entry: active_entry.as_ptr().cast::<()>(),
-                        remaining_repeat_count: sequence.remaining_repeat_count as i64,
+                        generation: sequence.generation,
                     }),
                 }
             }
@@ -1426,7 +1426,8 @@ bun_jsc::jsc_host_abi! {
 pub struct EntryData {
     pub(crate) sequence_index: usize,
     pub(crate) entry: *const (),
-    pub(crate) remaining_repeat_count: i64,
+    /// `ExecutionSequence::generation` at the time the entry was started.
+    pub(crate) generation: u32,
 }
 
 // Clone: bitwise OK — `active_scope` is a non-owning borrow of a
@@ -1493,8 +1494,8 @@ impl fmt::Display for RefDataValue {
                 if let Some(ed) = entry_data {
                     write!(
                         f,
-                        "execution: group_index={},sequence_index={},entry_index={:x},remaining_repeat_count={}",
-                        group_index, ed.sequence_index, ed.entry as usize, ed.remaining_repeat_count
+                        "execution: group_index={},sequence_index={},entry_index={:x},generation={}",
+                        group_index, ed.sequence_index, ed.entry as usize, ed.generation
                     )
                 } else {
                     write!(f, "execution: group_index={}", group_index)

test/js/bun/test/test-retry-repeats-basic.test.ts

@@ -1,8 +1,8 @@
-// Runs the retry/repeats hook-ordering checks in a subprocess so the
-// intentional intermediate retry failures don't leak into this run's
-// reporter output (JUnit, GitHub Actions annotations).
+// Every check here runs `bun test` in a subprocess so the intentional
+// intermediate retry failures don't leak into this run's reporter output
+// (JUnit, GitHub Actions annotations).
 import { expect, test } from "bun:test";
-import { bunEnv, bunExe } from "harness";
+import { bunEnv, bunExe, tempDir } from "harness";
 import { join } from "node:path";
 
 test("retry and repeats hook ordering", async () => {
@@ -20,3 +20,146 @@ test("retry and repeats hook ordering", async () => {
   expect(stderr).toContain("(attempt 3)");
   expect(exitCode).toBe(0);
 });
+
+async function runRetryFixture(name: string, source: string) {
+  using dir = tempDir(name, { "retry.test.ts": source });
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "test", "retry.test.ts"],
+    env: bunEnv,
+    cwd: String(dir),
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  return { stdout, stderr, exitCode };
+}
+
+// In each fixture below, attempt 1 times out while waiting on something that
+// attempt 2 completes as soon as it starts. Attempt 1's late completion must
+// not count as the completion of attempt 2.
+//
+// The ordering does not depend on timers: attempt 1's completion is queued while
+// attempt 2's body is still on the stack (synchronously by done(), or in the
+// microtask drain that follows the body), and the runner consumes it in the same
+// step, before anything attempt 2 scheduled can run. On an unfixed runner attempt
+// 2 is therefore reported as passed before its own timer fires; the timers only
+// keep attempt 2 alive past that point.
+
+test.concurrent("a late resolve from a timed-out attempt does not complete the retry", async () => {
+  const { stdout, stderr, exitCode } = await runRetryFixture(
+    "retry-stale-resolve",
+    `
+      import { test, expect } from "bun:test";
+      const first = Promise.withResolvers<void>();
+      let attempt = 0;
+      test("retry", async () => {
+        attempt++;
+        if (attempt === 1) {
+          await first.promise;
+          return;
+        }
+        first.resolve();
+        await Bun.sleep(1);
+        console.log("attempt 2 body finished");
+        expect(attempt).toBe(1);
+      }, { retry: 1, timeout: 500 });
+    `,
+  );
+
+  expect(stdout).toContain("attempt 2 body finished");
+  expect(stderr).toContain("(fail) retry (attempt 2)");
+  expect(stderr).toContain("Expected: 1");
+  expect(stderr).toContain("Received: 2");
+  expect(stderr).toContain("0 pass");
+  expect(stderr).toContain("1 fail");
+  expect(exitCode).toBe(1);
+});
+
+test.concurrent("a late resolve from a timed-out attempt keeps the retry's own timeout armed", async () => {
+  const { stderr, exitCode } = await runRetryFixture(
+    "retry-stale-resolve-timeout",
+    `
+      import { test } from "bun:test";
+      const first = Promise.withResolvers<void>();
+      let attempt = 0;
+      test("retry", async () => {
+        attempt++;
+        if (attempt === 1) {
+          await first.promise;
+          return;
+        }
+        first.resolve();
+        await new Promise(() => {});
+      }, { retry: 1, timeout: 100 });
+    `,
+  );
+
+  expect(stderr).toContain("(fail) retry (attempt 2)");
+  expect(stderr).toContain("this test timed out after 100ms");
+  expect(stderr).toContain("0 pass");
+  expect(stderr).toContain("1 fail");
+  expect(exitCode).toBe(1);
+});
+
+test.concurrent("a late done() from a timed-out attempt does not complete the retry", async () => {
+  const { stdout, stderr, exitCode } = await runRetryFixture(
+    "retry-stale-done",
+    `
+      import { test } from "bun:test";
+      let firstDone: (err?: unknown) => void;
+      let attempt = 0;
+      test("retry", done => {
+        attempt++;
+        if (attempt === 1) {
+          firstDone = done;
+          return;
+        }
+        firstDone();
+        setTimeout(() => {
+          console.log("attempt 2 body finished");
+          done(new Error("attempt 2 failed on its own"));
+        }, 1);
+      }, { retry: 1, timeout: 500 });
+    `,
+  );
+
+  expect(stdout).toContain("attempt 2 body finished");
+  expect(stderr).toContain("error: attempt 2 failed on its own");
+  expect(stderr).toContain("(fail) retry (attempt 2)");
+  expect(stderr).toContain("0 pass");
+  expect(stderr).toContain("1 fail");
+  expect(exitCode).toBe(1);
+});
+
+test.concurrent("a late rejection from a timed-out attempt is not attributed to the retry", async () => {
+  const { stdout, stderr, exitCode } = await runRetryFixture(
+    "retry-stale-reject",
+    `
+      import { test } from "bun:test";
+      const first = Promise.withResolvers<void>();
+      let attempt = 0;
+      test("retry", async () => {
+        attempt++;
+        if (attempt === 1) {
+          await first.promise;
+          return;
+        }
+        first.reject(new Error("late rejection from attempt 1"));
+        await Bun.sleep(1);
+        console.log("attempt 2 body finished");
+      }, { retry: 2, timeout: 500 });
+    `,
+  );
+
+  // Same as a timed-out test's promise rejecting while the next test runs: the
+  // error is reported between tests, and the running attempt is left alone.
+  expect(stdout).toContain("attempt 2 body finished");
+  expect(stderr).toContain("Unhandled error between tests");
+  expect(stderr).toContain("error: late rejection from attempt 1");
+  expect(stderr).toContain("(pass) retry (attempt 2)");
+  expect(stderr).not.toContain("(attempt 3)");
+  expect(stderr).toContain("1 pass");
+  expect(stderr).toContain("0 fail");
+  expect(stderr).toContain("1 error");
+  expect(exitCode).toBe(1);
+});