-
Notifications
You must be signed in to change notification settings - Fork 28.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test Runner add support for retries #48754
Comments
I wonder if it's not enough to put it on the describe context:
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:
|
That implementation seems problematic to me because:
As for:
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. |
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. |
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.
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 |
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 |
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. 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 |
@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? |
@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) |
@MoLow no, but we 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. |
This is one of the few missing features required to move jsdom from Mocha to Node's built-in test runner. |
@MoLow WDYT? |
I still think this is an anty-pattern and would personally use a userland implementation for retrying failures, but if one would implement a PR I won't block |
There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the
never-stale
|
not stale, worked on |
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 configurationretries: 1
, it will re-run the test one more time and if the second run succeeds, the test is considered as successful. Also all surroundingbeforeEach
andafterEach
scripts should be also executed again as they can contain some necessary setups and cleanups. By defaultretries
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
The same configuration can be also useful in
describe
andtest
/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.
The text was updated successfully, but these errors were encountered: