test_runner: add flaky option to retry on failure

[Han5991]

This feature is based on the flaky proposal in the nodejs/test-runner repo, which I used as the reference for the API surface and semantics. A test can be marked flaky so the runner re-runs it until it passes — useful for tests with unavoidable nondeterminism.

Supersedes #61746 (original work by @vespa7, credited as co-author); this is a fresh implementation on current main.

Closes #61746

What it adds

• flaky: true → retry up to 20 times; flaky: <positive integer> → explicit budget; flaky: false → off. Invalid values throw at registration.
• Works on tests and suites, plus the it.flaky / test.flaky / describe.flaky / suite.flaky shorthands. Nearest value wins (a test-case overrides its suite).
• Only the final attempt is observable: intermediate failures emit no test:fail, no per-test diagnostics, and nothing on the node.test error channel.
• New retryCount field on test:pass / test:fail / test:complete; reporters print a # FLAKY … directive; the run summary gains a flaky counter.
• beforeEach/afterEach re-run per attempt (state reset between retries); before/after run once. External abort and expectFailure are not retried.

Points I'd appreciate eyes on

I've left inline review comments on the specific lines that would most benefit from review — the flaky-scoped assertion-plan read, the subtest crash fix and its known limitation, and the run() process-isolation parent re-counting.

Testing

New test/parallel/test-runner-flaky.js with deterministic, counter-based fixtures, plus a reporter snapshot test. Full test_runner suite passes; lint clean.

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[Han5991]

A few self-review notes on the trickier parts, anchored to the exact lines.

[Han5991]

This is the one change that touches the non-flaky path: the assertion plan is now read at use-time, but only for flaky tests (test.flakyRetries > 0); non-flaky tests keep the registration-time capture (capturedPlan). Please confirm non-flaky plan counting is unchanged — the regression guard is the assert-before-plan case in test/parallel/test-runner-flaky.js.

[Han5991]

Subtest crash fix (1/2): on retry we clear subtests and reset outputSubtestCount. Without the reset the count stayed > 0 while subtests was emptied, which crashed the reporter (see the guard in 2/2).

[Han5991]

Subtest crash fix (2/2) + known limitation: this guards the empty-subtests dereference that aborted the run. Known limitation: intermediate-attempt subtests are emitted to the stream before the retry decision, so they cannot be recalled — the output can still contain leaked subtest lines and a slightly inflated # tests. Full fidelity would need buffering each attempt's events and replaying only the surviving one. Is the crash-fix-only version OK to land, with fidelity as a follow-up?

[Han5991]

run() process-isolation parent re-counts the child's serialized events, so two things are handled here: an exhausted flaky timeout (retryCount > 0) is promoted out of cancelled into a failure, and any flaky-marked test (retryCount present, even 0) counts toward flaky. This cross-process re-derivation is easy to get subtly wrong — please sanity-check.

[Han5991]

Timeout handling: a non-flaky timeout stays cancelled (#cancel), but a flaky one goes through fail() so it can retry and, when the budget is exhausted, counts as a failure rather than a cancellation. The flakyRetries === 0 guard is what preserves the existing non-flaky behavior.

[Han5991]

Semantics per the proposal: nearest-wins inheritance (this parent fallback), flaky: true → 20 retries (kDefaultFlakyRetries, L106; set just below), an explicit positive integer is used as-is, and 0/negative/non-integer values throw via validateInteger. Confirming these defaults match the proposal's intent would help.

[codecov]

Codecov Report

❌ Patch coverage is 93.72937% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.35%. Comparing base (30a7e28) to head (901c387).
⚠️ Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/reporter/junit.js	9.09%	10 Missing ⚠️
lib/internal/test_runner/test.js	97.57%	4 Missing and 2 partials ⚠️
lib/internal/test_runner/reporter/utils.js	86.66%	1 Missing and 1 partial ⚠️
lib/internal/test_runner/reporter/dot.js	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #63661      +/-   ##
==========================================
- Coverage   91.95%   90.35%   -1.61%     
==========================================
  Files         379      732     +353     
  Lines      166454   236910   +70456     
  Branches    25427    44631   +19204     
==========================================
+ Hits       153058   214050   +60992     
- Misses      13104    14574    +1470     
- Partials      292     8286    +7994     

Files with missing lines	Coverage Δ
lib/internal/test_runner/harness.js	88.15% <100.00%> (+0.02%)	⬆️
lib/internal/test_runner/reporter/tap.js	94.96% <100.00%> (+0.08%)	⬆️
lib/internal/test_runner/runner.js	94.51% <100.00%> (+0.39%)	⬆️
lib/internal/test_runner/tests_stream.js	92.64% <100.00%> (+0.14%)	⬆️
lib/internal/test_runner/utils.js	65.37% <100.00%> (+0.13%)	⬆️
lib/internal/test_runner/reporter/dot.js	97.72% <80.00%> (-2.28%)	⬇️
lib/internal/test_runner/reporter/utils.js	93.33% <86.66%> (-1.95%)	⬇️
lib/internal/test_runner/test.js	97.77% <97.57%> (-0.09%)	⬇️
lib/internal/test_runner/reporter/junit.js	89.08% <9.09%> (-5.40%)	⬇️

... and 494 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JakobJingleheimer]

Thanks for this!

A small thing (I think it wouldn't be much work to adjust for this): should it be tries instead of retries? Usually, a test is run N times inclusive of the initial. I think that would also simplify the implementation a bit too.

[JakobJingleheimer]

This doesn't do what it says, and I don't love that it creates side-effects (specifically via ArrayPrototypePush(ancestor.pendingCountedSubtests, this)).

I think a do...while is better suited here because we know the first time it's called that ancestor won't be empty.

Suggested change

	#countCompleted() {
	let ancestor = this.parent;
	while (ancestor !== null) {
	if (ancestor.isFlaky && ancestor.endTime === null) {
	ancestor.pendingCountedSubtests ??= [];
	ArrayPrototypePush(ancestor.pendingCountedSubtests, this);
	return;
	}
	ancestor = ancestor.parent;
	}
	/**
	* Climb up the tree, checking for pending tests.
	*/
	#collectPendingToRoot() {
	let ancestor = this.parent;
	do {
	if (ancestor.isFlaky && ancestor.endTime === null) {
	ancestor.pendingCountedSubtests ??= [];
	ArrayPrototypePush(ancestor.pendingCountedSubtests, this);
	return;
	}
	ancestor = ancestor.parent;
	}
	while (ancestor !== null);

Should the return be a break? Otherwise countCompletedTest never gets called, which it seems it should.

[Han5991]

I'd lean toward keeping the current shape:

• return (not break) is intentional: a subtest parked on a still-running flaky ancestor is re-counted when that ancestor reports (report() replays the parked list via pending[i].#countCompleted()). break would count it now and again on replay → double-counted # tests.
• while (not do...while): the root test reaches #countCompleted() with parent === null, where while skips the body and falls through to countCompletedTest(this). do...while would run the body once with ancestor === null and throw on ancestor.isFlaky.
• The ArrayPrototypePush is the mechanism (parking the count until the ancestor settles), not an incidental side effect.

If the comment reads as "doesn't do what it says," I'm happy to reword it to make the park-then-replay flow explicit — let me know which part was unclear.

[JakobJingleheimer]

Thanks for this!

A small thing (I think it wouldn't be much work to adjust for this): should it be tries instead of retries? Usually, a test is run N times inclusive of the initial. I think that would also simplify the implementation a bit too.

[Han5991]

should it be tries instead of retries? Usually, a test is run N times inclusive of the initial.

I went with retries-semantics (the number is additional attempts after the first run, so flaky: 3 → up to 4 runs) on purpose — it's what every major JS test runner does:

• Jest — retryTimes(n): "Runs failed tests n-times until they pass"
• Vitest — retry: "Number of times to retry a test if it fails" (default 0)
• Playwright — retries: 3: "Give failing tests 3 retry attempts"
• Cypress — retries: 2: "up to 2 additional times (for a total of 3 attempts)"
• Mocha — this.retries(4): retries a failed test up to 4 more times

Inclusive/total counts do exist, but they pair with names like attempts / invocationCount / @RepeatedTest, never with retry. Since this is a retry-on-failure feature (flakyRetries, retryCount in the reporter), retries-semantics matches the naming. Inclusive would also make flaky: 1 a valid no-op (1 run = 0 retries).

On the implementation side, inclusive only removes the + 1 in maxAttempts = flakyRetries + 1 — the loop and reporting are already attempt-based, so it isn't really a simplification. The docs already spell out the total ("1 initial run + 20 retries"), so the semantics is documented at the call site.

Happy to revisit if you feel strongly about inclusive counts.