Skip to content

fix: avoid unhandled promise rejections when concurrent tests fail#11987

Merged
SimenB merged 6 commits intojestjs:mainfrom
dmitri-gb:uncaught-rejection-fix
Nov 29, 2021
Merged

fix: avoid unhandled promise rejections when concurrent tests fail#11987
SimenB merged 6 commits intojestjs:mainfrom
dmitri-gb:uncaught-rejection-fix

Conversation

@dmitri-gb
Copy link
Copy Markdown
Contributor

@dmitri-gb dmitri-gb commented Oct 22, 2021

Summary

This change fixes an issue where a failing concurrent test could trigger the global unhandled promise rejection handler, which would then attribute this failure to a different test (the one that was currently "running" i.e. the one that Jest was currently awaiting on). The issue is present in both jest-jasmine2 and jest-circus.

You can also check existing bug reports (#11691, #11231) for examples.

Fixes #11691
Fixes #11231

Test plan

There is a new integration test that demonstrates the issue.

const promise = mutex(() => testFn());
// Avoid triggering the uncaught promise rejection handler in case the test errors before
// being awaited on.
promise.catch(() => {});
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this doesn't mean promise will never reject, that is only true for the promise returned by catch(...).

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 22, 2021

Codecov Report

Merging #11987 (589c9dc) into main (95f4969) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #11987      +/-   ##
==========================================
- Coverage   68.77%   68.76%   -0.01%     
==========================================
  Files         324      324              
  Lines       16670    16672       +2     
  Branches     4814     4814              
==========================================
  Hits        11465    11465              
- Misses       5172     5174       +2     
  Partials       33       33              
Impacted Files Coverage Δ
...us/src/legacy-code-todo-rewrite/jestAdapterInit.ts 0.00% <0.00%> (ø)
packages/jest-jasmine2/src/jasmineAsyncInstall.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95f4969...589c9dc. Read the comment docs.

@dmitri-gb dmitri-gb changed the title fix: avoid uncaught promise rejections when concurrent tests fail fix: avoid unhandled promise rejections when concurrent tests fail Oct 24, 2021
Copy link
Copy Markdown
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! I've been meaning to dig into these, thanks for taking the time 👍

@SimenB SimenB merged commit 9d14c5d into jestjs:main Nov 29, 2021
@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Dec 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

failures in test.concurrent tests cause unrelated tests to fail Test.Concurrent Causing False Negatives When One Test Throws an Error

4 participants