test_runner: fix rerun ambiguous test failures

MoLow · aduh95 · commit 9382be5b1607 · 2026-01-27T22:52:27.000+01:00
PR-URL: #61392 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
diff --git a/lib/internal/test_runner/reporter/rerun.js b/lib/internal/test_runner/reporter/rerun.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  ArrayPrototypeMap,
   ArrayPrototypePush,
   JSONStringify,
 } = primordials;
@@ -11,19 +12,55 @@ function reportReruns(previousRuns, globalOptions) {
   return async function reporter(source) {
     const obj = { __proto__: null };
     const disambiguator = { __proto__: null };
+    let currentSuite = null;
+    const roots = [];
+
+    function getTestId(data) {
+      return `${relative(globalOptions.cwd, data.file)}:${data.line}:${data.column}`;
+    }
+
+    function startTest(data) {
+      const originalSuite = currentSuite;
+      currentSuite = { __proto__: null, data, parent: currentSuite, children: [] };
+      if (originalSuite?.children) {
+        ArrayPrototypePush(originalSuite.children, currentSuite);
+      }
+      if (!currentSuite.parent) {
+        ArrayPrototypePush(roots, currentSuite);
+      }
+    }
 
     for await (const { type, data } of source) {
+      let currentTest;
+      if (type === 'test:start') {
+        startTest(data);
+      } else if (type === 'test:fail' || type === 'test:pass') {
+        if (!currentSuite) {
+          startTest({ __proto__: null, name: 'root', nesting: 0 });
+        }
+        if (currentSuite.data.name !== data.name || currentSuite.data.nesting !== data.nesting) {
+          startTest(data);
+        }
+        currentTest = currentSuite;
+        if (currentSuite?.data.nesting === data.nesting) {
+          currentSuite = currentSuite.parent;
+        }
+      }
+
+
       if (type === 'test:pass') {
-        let identifier = `${relative(globalOptions.cwd, data.file)}:${data.line}:${data.column}`;
+        let identifier = getTestId(data);
         if (disambiguator[identifier] !== undefined) {
           identifier += `:(${disambiguator[identifier]})`;
           disambiguator[identifier] += 1;
         } else {
           disambiguator[identifier] = 1;
         }
+        const children = ArrayPrototypeMap(currentTest.children, (child) => child.data);
         obj[identifier] = {
           __proto__: null,
           name: data.name,
+          children,
           passed_on_attempt: data.details.passed_on_attempt ?? data.details.attempt,
         };
       }
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -698,10 +698,18 @@ class Test extends AsyncResource {
         this.root.testDisambiguator.set(testIdentifier, 1);
       }
       this.attempt = this.root.harness.previousRuns.length;
-      const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[testIdentifier]?.passed_on_attempt;
+      const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[testIdentifier];
       if (previousAttempt != null) {
-        this.passedAttempt = previousAttempt;
-        this.fn = noop;
+        this.passedAttempt = previousAttempt.passed_on_attempt;
+        this.fn = () => {
+          for (let i = 0; i < (previousAttempt.children?.length ?? 0); i++) {
+            const child = previousAttempt.children[i];
+            this.createSubtest(Test, child.name, { __proto__: null }, noop, {
+              __proto__: null,
+              loc: [child.line, child.column, child.file],
+            }, noop).start();
+          }
+        };
       }
     }
   }
diff --git a/test/fixtures/test-runner/rerun.js b/test/fixtures/test-runner/rerun.js
@@ -22,4 +22,19 @@ function ambiguousTest(expectedAttempts) {
 }
 
 ambiguousTest(0);
-ambiguousTest(1);
+ambiguousTest(1);
+
+function nestedAmbiguousTest(expectedAttempts) {
+  return async (t) => {
+    await t.test('nested', async (tt) => {
+      await tt.test('2 levels deep', () => {});
+      if (t.attempt < expectedAttempts) {
+        throw new Error(`This test is expected to fail on the first ${expectedAttempts} attempts`);
+      }
+    });
+    await t.test('ok', () => {});
+  };
+}
+
+test('nested ambiguous (expectedAttempts=0)', nestedAmbiguousTest(0));
+test('nested ambiguous (expectedAttempts=1)', nestedAmbiguousTest(2));
diff --git a/test/parallel/test-runner-test-rerun-failures.js b/test/parallel/test-runner-test-rerun-failures.js
@@ -15,46 +15,74 @@ afterEach(() => rm(stateFile, { force: true }));
 
 const expectedStateFile = [
   {
-    'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
     'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
+    'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
+    'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' },
+    'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' },
+    'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' },
+    'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
+    'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
+    'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
   },
   {
+    'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
     'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
     'test/fixtures/test-runner/rerun.js:17:3:(1)': { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' },
-    'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
+    'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' },
+    'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' },
+    'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' },
+    'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
+    'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
+    'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
   },
   {
+    'test/fixtures/test-runner/rerun.js:3:1': { passed_on_attempt: 2, name: 'should fail on first two attempts' },
+    'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
     'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
     'test/fixtures/test-runner/rerun.js:17:3:(1)': { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' },
-    'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
-    'test/fixtures/test-runner/rerun.js:3:1': { passed_on_attempt: 2, name: 'should fail on first two attempts' },
+    'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' },
+    'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' },
+    'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' },
+    'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
+    'test/fixtures/test-runner/rerun.js:29:13:(1)': { passed_on_attempt: 2, name: 'nested' },
+    'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
+    'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
+    'test/fixtures/test-runner/rerun.js:40:1': { passed_on_attempt: 2, name: 'nested ambiguous (expectedAttempts=1)' },
   },
 ];
 
-const getStateFile = async () => JSON.parse((await readFile(stateFile, 'utf8')).replaceAll('\\\\', '/'));
+const getStateFile = async () => {
+  const res = JSON.parse((await readFile(stateFile, 'utf8')).replaceAll('\\\\', '/'));
+  res.forEach((entry) => {
+    for (const item in entry) {
+      delete entry[item].children;
+    }
+  });
+  return res;
+};
 
 test('test should pass on third rerun', async () => {
   const args = ['--test-rerun-failures', stateFile, fixture];
 
   let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args);
   assert.strictEqual(code, 1);
   assert.strictEqual(signal, null);
-  assert.match(stdout, /pass 2/);
-  assert.match(stdout, /fail 2/);
+  assert.match(stdout, /pass 8/);
+  assert.match(stdout, /fail 4/);
   assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1));
 
   ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
   assert.strictEqual(code, 1);
   assert.strictEqual(signal, null);
-  assert.match(stdout, /pass 3/);
-  assert.match(stdout, /fail 1/);
+  assert.match(stdout, /pass 9/);
+  assert.match(stdout, /fail 3/);
   assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2));
 
 
   ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
   assert.strictEqual(code, 0);
   assert.strictEqual(signal, null);
-  assert.match(stdout, /pass 4/);
+  assert.match(stdout, /pass 12/);
   assert.match(stdout, /fail 0/);
   assert.deepStrictEqual(await getStateFile(), expectedStateFile);
 });
@@ -65,30 +93,30 @@ test('test should pass on third rerun with `--test`', async () => {
   let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args);
   assert.strictEqual(code, 1);
   assert.strictEqual(signal, null);
-  assert.match(stdout, /pass 2/);
-  assert.match(stdout, /fail 2/);
+  assert.match(stdout, /pass 8/);
+  assert.match(stdout, /fail 4/);
   assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1));
 
   ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
   assert.strictEqual(code, 1);
   assert.strictEqual(signal, null);
-  assert.match(stdout, /pass 3/);
-  assert.match(stdout, /fail 1/);
+  assert.match(stdout, /pass 9/);
+  assert.match(stdout, /fail 3/);
   assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2));
 
 
   ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
   assert.strictEqual(code, 0);
   assert.strictEqual(signal, null);
-  assert.match(stdout, /pass 4/);
+  assert.match(stdout, /pass 12/);
   assert.match(stdout, /fail 0/);
   assert.deepStrictEqual(await getStateFile(), expectedStateFile);
 });
 
 test('using `run` api', async () => {
   let stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
-  stream.on('test:pass', common.mustCall(2));
-  stream.on('test:fail', common.mustCall(2));
+  stream.on('test:pass', common.mustCall(8));
+  stream.on('test:fail', common.mustCall(4));
 
   // eslint-disable-next-line no-unused-vars
   for await (const _ of stream);
@@ -97,8 +125,8 @@ test('using `run` api', async () => {
 
 
   stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
-  stream.on('test:pass', common.mustCall(3));
-  stream.on('test:fail', common.mustCall(1));
+  stream.on('test:pass', common.mustCall(9));
+  stream.on('test:fail', common.mustCall(3));
 
   // eslint-disable-next-line no-unused-vars
   for await (const _ of stream);
@@ -107,7 +135,7 @@ test('using `run` api', async () => {
 
 
   stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
-  stream.on('test:pass', common.mustCall(4));
+  stream.on('test:pass', common.mustCall(12));
   stream.on('test:fail', common.mustNotCall());
 
   // eslint-disable-next-line no-unused-vars

Original file line number	Diff line number	Diff line change
`@@ -698,10 +698,18 @@ class Test extends AsyncResource {`
`698`	`698`	`this.root.testDisambiguator.set(testIdentifier, 1);`
`699`	`699`	`}`
`700`	`700`	`this.attempt = this.root.harness.previousRuns.length;`
`701`		`- const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[testIdentifier]?.passed_on_attempt;`
	`701`	`+ const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[testIdentifier];`
`702`	`702`	`if (previousAttempt != null) {`
`703`		`- this.passedAttempt = previousAttempt;`
`704`		`- this.fn = noop;`
	`703`	`+ this.passedAttempt = previousAttempt.passed_on_attempt;`
	`704`	`+ this.fn = () => {`
	`705`	`+ for (let i = 0; i < (previousAttempt.children?.length ?? 0); i++) {`
	`706`	`+ const child = previousAttempt.children[i];`
	`707`	`+ this.createSubtest(Test, child.name, { __proto__: null }, noop, {`
	`708`	`+ __proto__: null,`
	`709`	`+ loc: [child.line, child.column, child.file],`
	`710`	`+ }, noop).start();`
	`711`	`+ }`
	`712`	`+ };`
`705`	`713`	`}`
`706`	`714`	`}`
`707`	`715`	`}`