Test Runner add support for retries

[Filipoliko]

What is the problem this feature will solve?

If the test runner is used for some integration, or end-2-end testing, there tends to be some flakiness because of the more complex setup relying on the stability of not just the tests, but also the environment that tests are running against.

What is the feature you are proposing to solve the problem?

Implement support for retries configuration. If a test fails and there is a configuration retries: 1, it will re-run the test one more time and if the second run succeeds, the test is considered as successful. Also all surrounding beforeEach and afterEach scripts should be also executed again as they can contain some necessary setups and cleanups. By default retries should be disabled as it is not something most users would like to have configured by default.

Reporters should also consider this possibility and could report on flaky/retried tests.

Inspiration can be taken from playwright documentation on retries.

CLI
Usage: node --test --test-retries=1 ./tests/test.js

TestRunner.run

import { run } from 'node:test';
import path from 'node:path';

run({
  files: [path.resolve('./tests/test.js')],
  retries: 1
})

The same configuration can be also useful in describe and test/it options object.

What alternatives have you considered?

I considered extending Node Test Runner with my own implementation of retries just for my use-case. But there does not seem to be an elegant solution how to accomplish this right now as the Test Runner does not seem to be very well extensible in this way.

The solution that would work, but wouldn't be very nice, is implementing wrapper function for the tests, that would handle retrying. But this would require wrapping all the test functions to be able to accomplish some global behaviour. Also Test Reporters would not display nice information about flakiness.

[benjamingr]

I wonder if it's not enough to put it on the describe context:

describe("something", (t) => {
  t.setRetries(4);
 // the rest
});

Which would enable per suite/test configuration and programmatic access from within the test file.

[MoLow]

I wonder if it's not enough to put it on the describe context:

describe("something", (t) => {
  t.setRetries(4);
 // the rest
});

Which would enable per suite/test configuration and programmatic access from within the test file.

I agree, and also personally prefer to just use a userland retry implementation, since:

• I consider that a bad practice for testing
• it is trivial to implement

const retry = (fn, retries = 5) => {
	try {
		await fn();
	} catch {
		if (retries > 0) {
			return rety(fn, retries - 1);
		}
	}
}
it("something", retry(() => {
 // the rest
}));

[benjamingr]

I agree, and also personally prefer to just use a userland retry implementation, since:

That implementation seems problematic to me because:

• The failures are not reported so the user isn't aware of flaky tests
• The failures can't be easily turned off globally or for a whole suite to detect flaky tessts.

As for:

I consider that a bad practice for testing

I mostly agree but I think this is a common enough use case that other test runners support and it's useful to run tests with retries as long as you go back and fix the flakiness.

If you look at our own code for example - we rerun flaky tests and fix them after all the time when we "resume ci". We just do it in a very manual/slow way.

[cjihrig]

I proposed this as a potential feature when originally writing the test runner, but received push back. I think this is one of those things that in theory most people agree you shouldn't do. The tests should be fixed instead. In practice though, as @benjamingr points out, it happens all the time. I'm not opposed to adding this feature.

[Filipoliko]

Regarding the user implementation of retries.. it seems trivial at first, but there are some problems you can run into, especially when implementing some long time running tests (i.e. with some browser setup and navigation). Some of them are mentioned by @benjamingr and I ran into some more.

1. Test timeout is shared for all retries (so if timeout for the test is 10 sec and the first run takes up that time, the second try is never executed)
2. Test fail error causes from previous retries are lost (which is very important to have to debug some flakiness)
3. beforeEach/afterEach scripts are not executed for each retry (this can make some retries doomed to fail before they even started)

This all can be still somehow implemented on the user side, but it is getting more and more complex and it is no longer the straightforward simple implementation mentioned by @MoLow

[ErickWendel]

I think this might be useful as a tool to check if a test suite is flaky.

Brainstorm - put a retry as 1000 times and output how many times it has failed - this could be amazing for debugging but I'm not sure about the effort for it

[MoLow]

I think this might be useful as a tool to check if a test suite is flaky.

that is a valid use case, but my guess is retries would be used as a footgun to just ignore/introduce flakiness not to debug it.
if it was totally impossible, I would agree adding it to core makes sense, but I still believe running this on user land is possible and not to complicated.

I am happy with this anty-pattern not being easy to use out of the box as long as there are decent alternatives in userland

[benjamingr]

@MoLow how can we say that while doing it ourselves in our own tests?

What about resume-ci like functionality where the test runner can take a list of failed tests in some format (tap or junit or whatever) and re-run just those failed tests?

[MoLow]

@benjamingr we don't use reruns in core in our tests. I'm just saying its a footgun that has many implementations in userland (some of we use)

[benjamingr]

@MoLow no, but we resume-ci a ton which is basically the same thing but more manual

When a PR fails for flakiness as most do we just rerun. We don't stop everything and look at the flakes they are gathered in the ci-reliability repo and dealt with asynchronously usually.

[domenic]

This is one of the few missing features required to move jsdom from Mocha to Node's built-in test runner.