jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.07k stars 6.44k forks source link

[jest-circus] suite level retry #10520

Open stkevintan opened 4 years ago

stkevintan commented 4 years ago

🚀 Feature Proposal

Support test suite level retry

Motivation

currently jest-circus only support re-run test cases according to https://github.com/facebook/jest/blob/23f425c15a5cdf80dd165a1b50c07b92edb8c10c/packages/jest-circus/src/run.ts#L69

however, in common e2e test scenario, some test cases are not isolated with others, they depend on the status of the previous test cases, so retry in test case level may be not helpful but suite level can

Example

maybe we can extend the jest.retryTimes method:

jest.retryTimes(TestCaseNumRetries: number, TestSuiteNumRetries?: number);

just a proposal, I am not good at design api. but if you guys think my proposal is doable, I'm glad to send a PR to implement it. 🤣

Pitch

Why does this feature belong in the Jest core platform?

Common feature proposals that do not typically make it to core:

oldwin commented 3 years ago

I totally agree with @stkevintan and looking for this feature too, can also add a bit more details why I need it.

As I understand the actual problem is running failed test at the end of suite. If you run

it('a)
it('b')
it('c')

And b fails, it will run a -> b (failed) -> c -> b (retry) I do not understand why we have this ordering, a -> b (failed) -> b (retry) -> c should solve a lot of possible problems inside suite.

Maybe we have a way to run it in this ordering right now? I didn't find it, but anyway @stkevintan request to rerun test suite will be very helpful, in my case I run browser tab for every test suite and tests dependce on prev browser state.

theseyi commented 3 years ago

As I understand the actual problem is running failed test at the end of suite. If you run

it('a)
it('b')
it('c')

And b fails, it will run a -> b (failed) -> c -> b (retry) I do not understand why we have this ordering, a -> b (failed) -> b (retry) -> c should solve a lot of possible problems inside suite.

This is better than the current implementation, i.e. re-running the failed case before proceeding to the next in the suite. However, ideally you want the option to re-run the entire suite if any case fails. This is especially useful in an e2e test context, where the entire suite tests a general workflow, with each case in the suite potentially causing side-effects e.g. mutating data or state that impacts later test cases. The suite will generally have a beforeAll that sets up and ensures a clean state, with an afterAll that cleans up.

So you would want something akin to beforeAll -> a -> b (failed) -> beforeAll -> a -> b -> c

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

vijay-zip commented 1 year ago

who ever sees this, I think jest is a unit testing tool, and we should no longer use jest for e2e tests; For unit tests the results are always black and white, no so much with e2e tests;

Jest circus has test retry, but it will not retry any BeforeAll block. Supertest - super agent also has request retry, but the logging is appalling. (we'd never know if something was retried unless we proxy and monitor the requests)

Overall just a bad experience so far trying to maintain e2e tests written ages ago using jest.

I'd recommend anyone to just start using playwright, it worked great in one of my previous engagements.

UPDATE 1: Some relief Actually in jest ^26.6.1, we can use: npx jest --onlyFailures , which will do a retry of all the failed tests, I know, the OP wanted the config at a suite level, but this --onlyFailures offers something. Its just that I need to pass a custom config, so that original reports are not overwritten.

WarpRat commented 1 year ago

@vijay-zip - does your custom config allow you to run --onlyFailures without breaking coverage thresholds? It would be really helpful for us to be able to use the --onlyFailures flag in our CI jobs to speed up retrying a flaky test but we can't right now without triggering coverage failures.

Right now I'm working around that with the following cli flag but it's brittle and hacky: --collectCoverageFrom "$(jq -r 'to_entries | map(select(.value[0] == 0)) | [.[].key]' .jest_cache/perf* | tr '\n' ' ' | sed 's/.spec//g' | sed 's|'\"${PWD}\"'|**|g')"`

noomorph commented 8 months ago

For all interested parties, you may try out this add-on:

https://github.com/wix-incubator/jest-retry-all-hooks

It monkey-patches beforeAll and afterAll hooks in a way that is more suitable for E2E tests.

cc @oldwin, @robvree, @theseyi, @c0d3d, @pauldraper, @vijay-zip, @pierrebeitz, @sirdir, @WarpRat, @MuckT, @stkevintan

SimenB commented 8 months ago

PR very much welcome adding support for this :+1:

noomorph commented 8 months ago

@SimenB do you think this implementation makes sense for the official Jest? I guess that the original author of Circus had some vision about describe-level hooks vs. test-level hooks, and this change would discard the notion of describe-level hooks. What are your thoughts on what the correct behavior has to be?

SimenB commented 8 months ago

I think we'd need a new option to retryTimes (maybe entireDescribe: true or some such) which attached it to the current describe rather than only rerunning failing tests.