fix: handle open handles from inside tests #6263

SimenB · 2018-05-25T10:11:39Z

Summary

@thymikee found this one before the tweet storm yesterday 😅

Test plan

Added test

SimenB · 2018-05-25T10:13:03Z

packages/jest-cli/src/cli/index.js

+
+    const stacks = new Set();
+
+    const uniqueErrors = formatted.filter(handle => {


this whole filter is really hairy. ideas?

The thought is to avoid 2 stack traces pointing to the same thing:

Any difference about Timeout and TIMERWRAP?

IDK. TIMEWRAP is a libuv thing, that's what I got from googling it. I don't think it really matters, they point to the same thing

Or maybe we can just ditch the filtering?

@cpojer wanted to filter out the dupe

Can you just add a comment about the dupes? Should be good enough

SimenB · 2018-05-25T10:17:44Z

packages/jest-jasmine2/src/jasmine_async.js

@@ -39,7 +39,7 @@ function promisifyLifeCycleFunction(originalFn, env) {

    // We make *all* functions async and run `done` right away if they
    // didn't return a promise.
-    const asyncFn = function(done) {
+    const asyncJestLifecycle = function(done) {


this seemed easiest 🤷‍♂️

thymikee

LGTM

thymikee · 2018-05-25T10:21:14Z

Some weird errors happening on Node 10

SimenB · 2018-05-25T10:27:22Z

Yeah, node 10.2.0 is messed up. nodejs/node#20921

SimenB · 2018-05-27T00:40:52Z

I need to fix this for circus

SimenB · 2018-05-27T09:49:29Z

@aaronabramov I added a second commit attempting to fix this for circus, but I still get some jest internals I don't care about. Current results is this:

Ideas on how to detect just stuff created in user code, and avoid the recursive calls circus itself does?

thymikee · 2018-05-27T09:52:35Z

packages/jest-cli/src/get_node_handles.js

-    if (error.stack.includes('Runtime.requireModule')) {
+    if (
+      error.stack.includes('Runtime.requireModule') ||
+      (error.stack.includes('callAsyncCircusFn') &&


shouldn't this be || instead of &&?

I pushed a new commit extracting this into a function where I think the logic (that doesn't really work...) is easier to follow

aaronabramov · 2018-05-27T13:37:47Z

wouldn't this always be triggered from somewhere in user's code though?
i'm not familiar with the whole error handling process and probably don't understand what's happening :(

aaronabramov · 2018-05-27T13:38:33Z

packages/jest-cli/src/cli/index.js

+
+    // E.g. timeouts might give multiple traces to the same line of code
+    // This hairy filtering tries to remove entries with duplicate stack traces
+    const uniqueErrors = formatted.filter(handle => {


should we exatract this into a separate module?
i a little worried about introducing so much complex logic right in index.js :P

yeah, I can extract it

SimenB · 2018-05-27T13:42:30Z

wouldn't this always be triggered from somewhere in user's code though?

Basically, we track absolutely everything async happening in the process. But a lot of async stuff is expected such as triggering describes and its (creates promises), so we should filter out those false positives so that we're only left with async stuff the user initiated

aaronabramov · 2018-05-27T13:59:48Z

@SimenB can we attach an extra flag to it when we trigger them?
like pass an asyncError with circus_internal = true?

thymikee · 2018-08-08T06:43:10Z

@SimenB internal promises should be resolved after #6716 is merged, right?

SimenB · 2018-08-08T06:43:52Z

Good point, that might actually solve my issue. That's awesome. I'll rebase after it lands 🙂

thymikee · 2018-08-08T19:59:56Z

@SimenB, just merged #6716

SimenB · 2018-08-09T12:01:23Z

Rebased, and looks good locally

SimenB · 2018-08-09T12:18:59Z

The dot reporter should include the normal summary as well... The test passes locally, no idea why it fails on CI.

SimenB · 2018-08-09T12:57:22Z

Hmm, for some reason this timeout is reported on CI, but not locally: https://github.com/facebook/jest/blob/c0819fc0c39dc3e7bc20513963678894e5bae6ab/packages/jest-circus/src/utils.js#L175-L178

SimenB · 2018-08-09T13:21:41Z

why-is-node-running removes TimerWrap, so I'll just do the same here.

https://github.com/mafintosh/why-is-node-running/blob/8f7908d31bd24b485cce218ce5552b8c75f24010/index.js#L10

SimenB · 2018-08-09T13:46:54Z

Green! 🎉

This reverts commit 176a9c1.

thymikee · 2018-08-09T15:15:04Z

🎉

github-actions · 2021-05-12T10:07:18Z

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.

SimenB commented May 25, 2018

View reviewed changes

thymikee approved these changes May 25, 2018

View reviewed changes

facebook-github-bot added the cla signed label May 25, 2018

SimenB force-pushed the open-handles-in-test branch 2 times, most recently from 3d28fb1 to 24606f3 Compare May 26, 2018 17:26

SimenB force-pushed the open-handles-in-test branch 3 times, most recently from a92dae2 to fed0667 Compare May 27, 2018 09:23

SimenB force-pushed the open-handles-in-test branch from d419c93 to f65ed9e Compare May 27, 2018 09:50

thymikee reviewed May 27, 2018

View reviewed changes

aaronabramov reviewed May 27, 2018

View reviewed changes

SimenB mentioned this pull request Jun 18, 2018

DNSCHANNEL stops jest from closing #6423

Closed

SimenB force-pushed the open-handles-in-test branch from 7eadb3d to 7756068 Compare July 17, 2018 10:31

SimenB mentioned this pull request Jul 19, 2018

Using lodash causes "open handles" warnings #6639

Closed

SimenB added 2 commits August 9, 2018 14:00

fix: handle open handles from inside tests

e53d7d3

[WIP] try to fix circus as well

860e923

SimenB force-pushed the open-handles-in-test branch from 7756068 to 860e923 Compare August 9, 2018 12:01

move hairy filtering out of index.js

41fcf39

report

176a9c1

ignore timerwraps as open handles

16e55fb

Revert "report"

216f2dd

This reverts commit 176a9c1.

SimenB merged commit 0099f07 into jestjs:master Aug 9, 2018

SimenB deleted the open-handles-in-test branch August 9, 2018 13:48

This was referenced Oct 1, 2018

Update jest in /modules/api from 22.4.3 to 23.6.0 project-accelerate/accelerate#245

Open

Update jest in /modules/frontend from 22.4.3 to 23.6.0 project-accelerate/accelerate#272

Open

dependencies bot mentioned this pull request Nov 1, 2018

Update jest in / from 22.1.4 to 23.6.0 leonardofaria/leozera-ui#250

Closed

snyk-bot mentioned this pull request Mar 17, 2020

[Snyk] Upgrade expect from 23.0.0 to 23.6.0 yowainwright/react-workshop#8

Merged

dependabot bot mentioned this pull request Mar 13, 2021

Bump babel-jest from 18.0.0 to 23.6.0 CartoonIsArt/eevee#133

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: handle open handles from inside tests #6263

fix: handle open handles from inside tests #6263

SimenB commented May 25, 2018

SimenB May 25, 2018 •

edited

thymikee May 25, 2018 •

edited

SimenB May 25, 2018

thymikee May 25, 2018

SimenB May 25, 2018

thymikee May 25, 2018

SimenB May 25, 2018

SimenB May 25, 2018

thymikee left a comment

thymikee commented May 25, 2018

SimenB commented May 25, 2018

SimenB commented May 27, 2018

SimenB commented May 27, 2018

thymikee May 27, 2018

SimenB May 27, 2018 •

edited

aaronabramov commented May 27, 2018

aaronabramov May 27, 2018

SimenB May 27, 2018

SimenB Aug 9, 2018

SimenB commented May 27, 2018

aaronabramov commented May 27, 2018

thymikee commented Aug 8, 2018

SimenB commented Aug 8, 2018

thymikee commented Aug 8, 2018

SimenB commented Aug 9, 2018

SimenB commented Aug 9, 2018 •

edited

SimenB commented Aug 9, 2018 •

edited

SimenB commented Aug 9, 2018 •

edited

SimenB commented Aug 9, 2018

thymikee commented Aug 9, 2018

github-actions bot commented May 12, 2021


		const stacks = new Set();

		const uniqueErrors = formatted.filter(handle => {

fix: handle open handles from inside tests #6263

fix: handle open handles from inside tests #6263

Conversation

SimenB commented May 25, 2018

Summary

Test plan

SimenB May 25, 2018 • edited

Choose a reason for hiding this comment

thymikee May 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thymikee left a comment

Choose a reason for hiding this comment

thymikee commented May 25, 2018

SimenB commented May 25, 2018

SimenB commented May 27, 2018

SimenB commented May 27, 2018

Choose a reason for hiding this comment

SimenB May 27, 2018 • edited

Choose a reason for hiding this comment

aaronabramov commented May 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB commented May 27, 2018

aaronabramov commented May 27, 2018

thymikee commented Aug 8, 2018

SimenB commented Aug 8, 2018

thymikee commented Aug 8, 2018

SimenB commented Aug 9, 2018

SimenB commented Aug 9, 2018 • edited

SimenB commented Aug 9, 2018 • edited

SimenB commented Aug 9, 2018 • edited

SimenB commented Aug 9, 2018

thymikee commented Aug 9, 2018

github-actions bot commented May 12, 2021

SimenB May 25, 2018 •

edited

thymikee May 25, 2018 •

edited

SimenB May 27, 2018 •

edited

SimenB commented Aug 9, 2018 •

edited

SimenB commented Aug 9, 2018 •

edited

SimenB commented Aug 9, 2018 •

edited