fix(jest): basic --bail support #4115
Conversation
noomorph
force-pushed
the
hotfix/jest-bail
branch
from
ec82bff
to
de6b982
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
noomorph
changed the title
noomorph
changed the title
|
The pull request has been tested via integration and extensive manual testing, so merging it. |
| // a bug in Jest, where `onRunComplete` is called multiple times when | ||
| // Jest runs with `--bail` and multiple workers, and `onRunComplete` | ||
| // is implemented as an asynchronous long-running operation. | ||
| this._lastRunComplete = this._pendingTestFiles.size === 0 |
There was a problem hiding this comment.
OK so as a name, _lastRunComplete is kind of confusing. It needs a name that better conveys the meaning behind it, namely of "aggregated results of the last call to onRunComplete()" - I reckon? WDYT?
There was a problem hiding this comment.
In any case, the flow here if incredibly hard to follow. Jest's fault, I know - but we need this thing somehow simplified. On the one hand, onRunComplete is triggered multiple times, awaiting the currently set _lastRunComplete. On the other, onTestFileResult only resolves the last _lastRunComplete (lol). What flow is being mitigated here, really?... I saw some of that is described in the issue's main description, but ideally, the code itself should convey the complete picture, regardless (or at least with complimentary commenting work).
|
|
||
| if (earlyTeardown && lostTests > 0 && config.testRunner.retries > 0) { | ||
| console.warn( | ||
| 'Jest aborted the test execution before all scheduled test files have been reported.\n' + |
There was a problem hiding this comment.
TL;DR? 😅 Can this be made more concise, perhaps by replacing some of the description with the link to the github issue in Jest? 🤔
| // @ts-expect-error TS2554 | ||
| super.onRunComplete(testContexts, aggregatedResult); | ||
|
|
||
| if (earlyTeardown && lostTests > 0 && config.testRunner.retries > 0) { |
There was a problem hiding this comment.
I feel like this should be nested under if (earlyTeardown) { of line #93
Description
Related to jestjs/jest#14143
This PR attempts to bridge the challenging integration between Jest's
-bail, Detox's--retries, and the intertwined bugs and constraints. Here's a brief overview of the obstacles addressed:DetoxReporter#onRunCompleteinvocation: Ensure it's called only once using cached promises and deferreds. Concurrent failures in other workers might trigger redundant invocations of the same callback.numTotalTestSuitesis greater than the count of known test files. In such cases, critical information for scheduling another re-run via Detox's--retriesbecomes unavailable.globalTeardownManagement: Ensure our global teardown is executed. Given Jest's apathy towards this, we manually invoke Detox's internalcleanup()function during early exits.While these interventions significantly improve the use of Jest's --bail, a notable limitation remains detailed in point (2). The best we can do is print a warning and give up on retrying.