Skip to content
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

Jest process doesn’t quit after last test completes #1456

Closed
calebmer opened this issue Aug 18, 2016 · 66 comments
Closed

Jest process doesn’t quit after last test completes #1456

calebmer opened this issue Aug 18, 2016 · 66 comments

Comments

@calebmer
Copy link
Contributor

I’m having issues with the Jest process not completing after the last test completes. The user will have to force quit the process with ctrl-c. My theory is that not all resources are being cleaned up appropriately by the test authors, but ideally Jest should quit anyway.

Specifically I’m testing Firebase with firebase-server, spinning up one or more servers for every test. In an afterEach action we call the close method for all the servers created in the last test, however even with this method the Jest process still doesn’t quit.

Is there a way to force the Jest process to quit once tests have finished (pass or fail)? Is there a way to get an afterAll hook to cleanup all leftover resources? Is there a way to debug what exactly keeps the Jest process from quitting? Thanks.

@cpojer
Copy link
Member

cpojer commented Aug 18, 2016

We don't have a good way to do that right now. I would recommend trying to hook in a debugger (Chrome Inspector) to figure out what is going on. If you know what creates the async work, you can potentially also monkey patch it and track it (like put something around Promise.prototype.then)

@calebmer
Copy link
Contributor Author

Is there a reason async work can’t be force quit when all afterEach/after hooks have resolved?

@cpojer
Copy link
Member

cpojer commented Aug 18, 2016

I don't know how you'd kill existing async processes if you don't have a handle on them.

@calebmer
Copy link
Contributor Author

I migrated from ava and this wasn’t a problem, so maybe there is an answer? Maybe that’s process.exit in Node.js

@cpojer
Copy link
Member

cpojer commented Aug 18, 2016

We could do that, I guess, but I'm worried it leaves people not hanging when it should and they aren't shutting down their resources properly.

cc @DmitriiAbramov what do you think?

@cpojer
Copy link
Member

cpojer commented Aug 18, 2016

One example: I actually ran into this with Jest myself where we had a long running watcher process that wouldn't terminate Jest itself. If Jest would have killed itself during the test run (heh!) I would have never noticed the issue and I would have shipped a version that would hang when people try to use it.

@aaronabramov
Copy link
Contributor

i'm not sure if it's safe to force kill the process. at the same time, if people want to do some async post processing after the tests are done, they could use after all hook that will wait for it to finish before quitting the process.

the other issue that we might have is cutting the output stream before it finished printing. we had this issue before, when error messages don't have enough time to print before the process exits.

@cpojer
Copy link
Member

cpojer commented Aug 18, 2016

The question is whether we could figure out a way for Jest to say "Looks like some tests aren't cleaning up after themselves. here is what is going on".

@calebmer
Copy link
Contributor Author

An after all hook could help me here, but I haven’t seen such a thing in the documentation (only afterEach) am I missing anything?

As for testing correct cleanups, if you could test if files were completing on time and if they weren’t using a bisect feature to isolate the problem (like in rspec).

@calebmer
Copy link
Contributor Author

Ok, so after more research this appears to be a problem with Firebase itself, and there is no way to cleanup short of calling process.exit.

Resources:

All of the workarounds involve manually calling process.exit. I’m afraid to do this in a Jest context, is there a recommendation on where to put such a call? My first thought was something like:

afterAll(() => setTimeout(() => process.exit(), 1000))

…to exit one second after all tests finished running to let Jest do its thing, however I’m not sure how this affects watch mode, and if I’m correct Jest does some fancy parallelism stuff which might make this not work as expected. Alternatively, would this be something you need to fix in Jest proper? If this seems like a footgun for a number of people, why not put it into Jest? Or at least a toggle between “warn” mode and “kill” mode.

@jonathanong
Copy link

i'd love a --exit flag or something (it could be a per-file comment or something) that automatically closes the processes when tests complete, similar to mocha. it's a little annoying to manually close every connection in every test file.

@gillchristian
Copy link

gillchristian commented Sep 12, 2016

I'm having the same problem when I run tests in Codeship. It also fails on drone.io.
Locally it works properly though.

EDIT:

  • Jest version: 15.1.1
  • Local:
    • Node: 6.2.2
    • npm: 3.9.5
  • CodeShip:
    • Node: 6.5.0
    • npm: 3.10.3
  • Drone.io
    • Node: 0.10.26
    • npm: 1.4.3

@cpojer
Copy link
Member

cpojer commented Sep 12, 2016

It seems to me like Firebase should be fixed.

I have no reservations against adding an option called --forceExitAfterTestRun and it should be easy to add. I think it just requires a change to exit here: https://github.com/facebook/jest/blob/master/packages/jest-cli/src/cli/index.js#L41 if the flag is given regardless of the result.

@4ware
Copy link

4ware commented Sep 12, 2016

Seems to be a race condition of sorts. Sometimes it quits after running all test locally sometimes it doesn't...

@jorilallo
Copy link

I'm running this as well after starting to use Jest for my API specs where I'm using real database instead of mocks (sorry 😇 but snapshots are great for this). I yet been able to solve the issue even after adding afterAll hooks to clean up connections which leads me to believe it's some how related to my population of fixtures in setupFiles, not the easiest to debug.

Jasmine seems to have --forceexit option so I would not complain if similar would also land to Jest 🙏

@jonathanong
Copy link

another issue - if a test fails, then afterAll() isn't called, so nothing is cleaned up and nothing closes. i think --bail will fix this but i haven't tried that yet

@cpojer
Copy link
Member

cpojer commented Sep 15, 2016

If anyone would like to send a PR, this is something that we could use some help with and I outlined details in my previous comment :)

@gillchristian
Copy link

I'll give it a shot if I get some time over the weekend. If someone wants to do it before that it'd cool 😄

yutin1987 added a commit to yutin1987/jest that referenced this issue Oct 3, 2016
After run all tests that force exit when forceExitAfterTestRun is true

jestjs#1456
@cpojer
Copy link
Member

cpojer commented Oct 3, 2016

Closing in favor of the PR that was just opened. We'll continue the discussion there.

@cpojer cpojer closed this as completed Oct 3, 2016
yutin1987 added a commit to yutin1987/jest that referenced this issue Oct 3, 2016
After run all tests that force exit when forceExitAfterTestRun is true

jestjs#1456
yutin1987 added a commit to yutin1987/jest that referenced this issue Oct 3, 2016
After run all tests that force exit when forceExit is true

jestjs#1456
pastuxso added a commit to pastuxso/pokedom that referenced this issue Oct 5, 2016
Add patchet jest-cli with the command line param --forceExit to solve
problems when jest process doesn’t quit after last test completes.

More info:

jestjs/jest#1456
cpojer pushed a commit that referenced this issue Oct 17, 2016
…1847) (#1870)

* feat(jest-cli): add forceExit avg on cli

After run all tests that force exit when forceExit is true

#1456

* Fix typecheck & linter offences

* Improve null check expression

* Update index.js
@distractdiverge
Copy link

So.... I was fighting with this for quite some time (using travis ci, coveralls and typescript).
It would end up hanging and producing a failed build (but only w/in Travis CI).

After much trial and error, i found what fixed this for me was that I added in an explicit path to the tests.

It failed with the npm script

   "test": "jest",
    "test:coverage": "npm run test -- --collectCoverage && cat ./src/coverage/lcov.info | coveralls",

And passed (in travis ci) with:

   "test": "jest .*\.test\.ts",
    "test:coverage": "npm run test -- --collectCoverage && cat ./src/coverage/lcov.info | coveralls",

gnprice added a commit to zulip/zulip-mobile that referenced this issue May 25, 2019
This reverts #3288, aka commit 7ed7e52 (and so re-opens #3281.)

After this change, Jest would hang after all tests had completed.
(Not sure how I missed that before merge -- did I neglect to run the
tests? >_>)  I noticed the problem a few commits later, and it took a
bit of debugging to identify this test file, and then this commit, as
the culprit.

Some background I found helpful in debugging: jestjs/jest#1456 .

The root of the issue seems to be that a lot of these tests fire off
`fetchMessages` actions, or others that in turn invoke that one -- and
then don't wait for them.  Compounding that, many of them are doomed
to fail at their HTTP requests because their fixture states have no
accounts, so there's no realm to base the URLs on.

Anyway, we don't knowingly merge changes that break things.  Merging
this was a mistake; take it back out.  We can merge a version of this
change once we have one that doesn't cause breakage.
@germanattanasio
Copy link

germanattanasio commented Jul 30, 2019

If you are using docker with a Redhat UBI image and create-react-app make sure you set CI=true before running npm test

@mrbinky3000
Copy link

mrbinky3000 commented Dec 20, 2019

December 2019. Ran into this issue ONLY on Travis. Tests pass locally. @qopqopqop 's fix worked for me. Using Jest version 24.9.0

I only encountered this error when our project started adding new components that use hooks and testing-library. I think there may be some friction between Jest, testing-library, and React hooks as these are all new technologies. These projects are still learning how to play nicely with each other. OR, we could be writing really buggy functional components that use hooks improperly. :-)

@kopax
Copy link

kopax commented Dec 24, 2019

I still have this issue. I can't exit the test, this make npm test fail for all my app. any clue?

@srfrnk
Copy link

srfrnk commented Dec 24, 2019

@koooge Can you post an example of what doesn't work for you?

@noelukwa
Copy link

To make the test exit with 0 after all tests pass you have to pass in --watchAll=false
like npm run test -- --watchAll=false

this worked for me

@christianeide
Copy link

To make this work with Firebase I had to do this:

afterAll(() => {
	firebase.app().delete();
});

@wgrisa
Copy link

wgrisa commented Mar 18, 2020

I've fixed it using the option:

--forceExit

https://jestjs.io/docs/en/cli#--forceexit

@toddbluhm
Copy link

toddbluhm commented Apr 5, 2020

So I ran into this issue as well. It was annoying seeing the A worker process has failed to exit gracefully and has been force exited... warning message when I knew I was handling all of my async calls correctly. I ran my tests with the --detectOpenHandles but nothing showed up. After some research, I discovered that my culprit was Promise.race.

I use a native promise utility library (https://github.com/blend/promise-utils) and wrap some of my external API calls in the timeout utility. This utility in-turn uses the native Promise.race.

I pulled that code out and created a simple test case to confirm my findings.

it('promise.race', async() => {
  await Promise.race([
    new Promise((res) => setTimeout(res, 10000)),
    Promise.resolve('true')
  ])
})

Assuming your test case timeout settings are default, the above test will always give the warning.

Whatever way jest is using to detect open handles under the hood, it is not taking into consideration handles left open intentionally by Promise.race. This use case definitely falls under the false-positive category. I am not sure this false-positive is fixable, but perhaps one of the devs has an ingenious solution to this.

For now, I am sticking with --forceExit like everyone else.

Edit:
Just found this, seems like it is indeed a deeper nodejs/v8 issue nodejs/node#24321

@garyo
Copy link

garyo commented May 28, 2020

For anyone else coming here from Firestore tests, this works for me:

afterAll(async () => {
  // Shut down Firestore, otherwise jest doesn't exit cleanly
  await firestoreInstance.terminate()
});

@alfasin
Copy link

alfasin commented Sep 5, 2020

I'm still having the same problem using Apollo & Jest. Unfortunately, even though the option --detectOpenHandles eventually exits, it still makes the process pend for another few seconds (and in contradiction to its name: it doesn't provide information about which handles are still open!).

Using --forceExit does the job but annoyingly prints:

Force exiting Jest: Have you considered using --detectOpenHandles to detect async operations that kept running after > all tests finished?

A workaround I found (and this is by no means a solution!) is adding a teardown to jest.config.js:

globalTeardown: '<rootDir>/__tests__/teardown.js',

and in teardown.js use process.exit:

module.exports = async function () {
    console.log('done!');
    process.exit(0);
}

@aminya
Copy link

aminya commented Oct 18, 2020

I also have this problem. How can I fix it? I have set forceExit: true. --forceExit --detectOpenHandles --maxWorkers=10 does not work.

atom-community/atom-ide-base#33

Edit: the issue somewhere else. It is in the test-runner that I am using...

@s2zaman
Copy link

s2zaman commented Nov 11, 2020

@alusicode
This one did not work for me npm test --watchAll=false
But it worked by adding --watchAll=false in package.json file. 👍

Like

"test": "react-scripts test a jest --ci --reporters=default --reporters=jest-junit --watchAll=false"

Official docs: https://jestjs.io/docs/en/cli.html#--watchall

@isengartz
Copy link

Not using firebase but I had the same issue on my workflow scripts. using jest without parameters said the some of my scripts didnt shutdown gracefully and I should use --runInBand --detectOpenHandles . That would fix the problem for all of my tests except one ( btw --detectOpenHandles didnt show me the tests that had issues) .
So I started checking all tests one by one. I found out that for 2 tests I forgot to use await when I was calling an async function.
After adding await it was fixed. Although I dont think its normal that --detectOpenHandles doesnt print the issues.

@fsevenm
Copy link

fsevenm commented Dec 15, 2020

I'm still having the same problem using Apollo & Jest. Unfortunately, even though the option --detectOpenHandles eventually exits, it still makes the process pend for another few seconds (and in contradiction to its name: it doesn't provide information about which handles are still open!).

Using --forceExit does the job but annoyingly prints:

Force exiting Jest: Have you considered using --detectOpenHandles to detect async operations that kept running after > all tests finished?

A workaround I found (and this is by no means a solution!) is adding a teardown to jest.config.js:

globalTeardown: '<rootDir>/__tests__/teardown.js',

and in teardown.js use process.exit:

module.exports = async function () {
    console.log('done!');
    process.exit(0);
}

I agree that this maybe is not a solution for the issue but at least it saves me for now.

@Ciph3rzer0
Copy link

Ciph3rzer0 commented Jan 3, 2021

I agree that this maybe is not a solution for the issue but at least it saves me for now.

Why does this work? I'm using firebase emulator for unit tests and I think this bit solved the last of my problems. I've tried all these and then some in afterAll, but they weren't enough:

    await Promise.all(firebase.apps().map((app) => app.delete()));
    await firebase.firestore().disableNetwork();
    await firebase.firestore().terminate();

I'm glad the warnings are gone anyways.

EDIT: This doesn't solve the problem in --watch mode. Very unfortunate.

@stormmuller
Copy link

I had to remove --watch I added it to my npm script while testing locally and checked it into CI and then it hung

@aseidma
Copy link

aseidma commented Feb 6, 2021

Something I noticed during debugging was that it does not seem to be the firebase/firestore instance triggering the error, but rather the assertions.

The following code shows the warning after running the test suite:

/**
 * @jest-environment node
 */

import * as firebase from "@firebase/rules-unit-testing"

let db: any
const user = {
	uid: "some-uid",
	displayName: "user",
}

beforeEach(() => {
	firebase.initializeTestApp({
		projectId: "project-id",
		auth: user,
	})
	db = firebase.apps()[0].firestore()
})
describe("Firestore Security Rules", () => {
	it("allows user to get his own modules", async () => {
		const moduleRequest = db
			.collection("users")
			.doc(user.displayName)
			.collection("modules")
			.doc("docID")
		await firebase.assertSucceeds(moduleRequest.get())
	})
})

afterAll(async () => {
	await db.terminate()
	await Promise.all(firebase.apps().map((app) => app.delete()))
})

However if you remove the assertion in the test, it exits just fine:

/**
 * @jest-environment node
 */

import * as firebase from "@firebase/rules-unit-testing"

let db: any
const user = {
	uid: "some-uid",
	displayName: "user",
}

beforeEach(() => {
	firebase.initializeTestApp({
		projectId: "project-id",
		auth: user,
	})
	db = firebase.apps()[0].firestore()
})
describe("Firestore Security Rules", () => {
	it("allows user to get his own modules", async () => {
		const moduleRequest = db
			.collection("users")
			.doc(user.displayName)
			.collection("modules")
			.doc("docID")
		// await firebase.assertSucceeds(moduleRequest.get())
	})
})

afterAll(async () => {
	await db.terminate()
	await Promise.all(firebase.apps().map((app) => app.delete()))
})

Changing the beforeEach hook to beforeAll (just to be safe the app and db is initialized) still exits without an issue.
I therefore suspect firebase.assertSucceeds does not exit a process properly which leads to jest not being able to exit.

@DCastaban
Copy link

Had the same issue, fixed it with a comment earlier in the thread:
npm run test -- --ci --watchAll=false

it will not wait for interactive input. You can add this to your package.json with:
react-scripts test a jest --ci --watchAll=false

@github-actions
Copy link

This issue 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 May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests