mocha: mocha 4 doesn't exit unlike mocha 3
Prerequisites
- [x ] Checked that your issue isn’t already filed by cross referencing issues with the
common mistake
label - Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn’t just a feature that actually isn’t supported in the environment in question or a bug in your code.
- [ x] ‘Smoke tested’ the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
- [ x] Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with:
node node_modules/.bin/mocha --version
(Local) andmocha --version
(Global). We recommend avoiding the use of globally installed Mocha.
Description
I have been running specific set of tests for few years now and have been always upgrading to the latest mocha and everything was ok. With mocha 4 suddenly all tests are passing but it doesn’t end as if the --no-exit is automatically added although I never added it.
Steps to Reproduce
Expected behavior: After all tests end, the process should stop even if there are timeoutes or sockets that prevent the process from existing.
Actual behavior: Mocha 4 process waits forever like mocha 3 with --no-exit flag
Reproduces how often: With our tests always. I have 700 tests so it is hard to pinpoint which one casuses the issue or if maybe it is in our codebase.
Versions
mocha 4.0.0 fails. before that everything works good.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 70
- Comments: 61 (15 by maintainers)
Commits related to this issue
- package.json: exit mocha after tests are done; mochajs/mocha#3044 Signed-off-by: Mario Kozjak <kozjakm1@gmail.com> — committed to elpheria/rpc-websockets by mkozjak 7 years ago
- Upgrade to latest mocha (4.0.1) Previous attempts to upgrade failed because testserver stayed open; this fixes that https://mochajs.org/#--exit----no-exit https://boneskull.com/mocha-v4-nears-release/... — committed to w3c/echidna by tripu 7 years ago
- Clean up request-handler, github-auth, and analytics; upgrade to Mocha 4 (#1142) - Add tests to request-handler to prepare for some tweaks to caching for #820 - Clean up code in request-handler: ren... — committed to badges/shields by paulmelnikow 7 years ago
- Close server after tests Fixes https://github.com/chaijs/chai-http/issues/178, caused by https://github.com/mochajs/mocha/issues/3044. — committed to ampproject/error-tracker by jridgewell 7 years ago
- Update dependencies to enable Greenkeeper 🌴 (#27) * chore(package): update dependencies * docs(readme): add Greenkeeper badge * Update lockfile * Close server after tests Fixes https://g... — committed to ampproject/error-tracker by greenkeeper[bot] 7 years ago
- Ensure tests exit in Travis refs https://github.com/mochajs/mocha/issues/3044 — committed to kirrg001/Ghost by kirrg001 7 years ago
- Ensure tests exit in Travis (#9288) refs https://github.com/mochajs/mocha/issues/3044 - there was a breaking change in mocha, which i have mentioned [here](https://github.com/TryGhost/Ghost/commit... — committed to TryGhost/Ghost by kirrg001 7 years ago
- fix(memory-utilization): don't keep bounded reference of connection manager (#8760) — committed to sequelize/sequelize by sushantdhiman 7 years ago
- Add --exit to all mocha calls so they exit when called from top-level yarn command (https://github.com/mochajs/mocha/issues/3044) — committed to 0xProject/0x-monorepo by fabioberger 6 years ago
- Update dependencies Also tweak test to terminate even with mocha 4+ [1]. [1] https://github.com/mochajs/mocha/issues/3044 — committed to mtth/avsc by mtth 6 years ago
- Fix tests with workaround, with mocha --exit param. https://github.com/mochajs/mocha/issues/3044 — committed to Nucleus-Inc/MeanStarter by Igor-Lopes 6 years ago
- chore: We can now upgrade to mocha@5 :taco: ## Symptoms Upgrading to `mocha@5` now causes my tests to hang without any context. [See this Travis build as an example](https://travis-ci.org/mongodb-js... — committed to mongodb-js/data-service by imlucas 5 years ago
- Add --exit flag to mocha command line to force exit. Since v4 mocha doesn't force-exit if a process or a socket is still running. https://github.com/mochajs/mocha/issues/3044 — committed to pryv/example-service-bluebutton by loic-pryv 5 years ago
- add exit https://github.com/mochajs/mocha/issues/3044 — committed to veraele/mean by jusech5 4 years ago
Other than providing a quick fix (use
--exit
), I do agree they left the core issue of finding the faulty tests to the user. I’m struggling with that myself now, but when upgrading major versions, I make sure to read the release notes and not blindly upgradeDid you guys read the release notes? https://boneskull.com/mocha-v4-nears-release/#mochawontforceexit
I suppose my general advice would be “if you’re pulling your hair out, just add
--exit
and relax”. Then don’t use it for your next project 😉I’m sorry, but this was very frustrating. I was just trying to find out why mocha won’t exit. https://boneskull.com/mocha-v4-nears-release/#mochawontforceexit links to
why-is-node-running
, so now I have gone, tried to use that module, seen a cryptic message “Error: Cannot find module ‘internal/linkedlist’ … (stack trace)”, and after following the rabbit hole trying to find out why --expose-internals doesn’t work, I wind up at https://github.com/mochajs/mocha/issues/3045 to find a supposed solution, butwhy-is-node-running
tells me that there is 1 known handle keeping it open, and a few unknown handles, but doesn’t list anything.This might not be the right place to bring this up, but mocha’s docs shouldn’t be recommending solutions that require this much googling and issue diving to get to work. I was trying taking things out and adding them in, with very inconsistent behavior. For the sake of future users and my sanity:
package.json:
That’s the best I can do.
Hi all,
First off, I want to apologize for the rough upgrade path on this point – we definitely agree that Mocha ought to tell the user what’s keeping the tests running (and then it wouldn’t even need to keep the process open; it could just be a reason to return failure after they’re done), but haven’t found an entirely satisfactory way to do so yet. Version 4 didn’t get the amount of time we wanted to put into it, as it was prompted by our CI failing due to changes in PhantomJS 1.x’s installer (package-lock.json probably would have prevented this if we’d had it set up beforehand, but we still can’t get it working).
why-is-node-running
was the one tool we found to help, but we don’t think we can integrate it (between the requirement of--expose-internals
and lack of a good way to programmatically get its output); I have found that it does work if you follow a couple steps:--expose-internals
(node --expose-internals node_modules/mocha/bin/_mocha
)mocha.opts
) contain (or at least begin with)after(require('why-is-node-running'))
…so it’s better than nothing (although I’ll see if I can update the release notes to describe this in more detail), but if anyone knows a better option please let us know!
Also sorry about missing the site documentation – we’ll get it updated as soon as possible. (Once #2987 is done we can even make it an automatic part of our version/publish scripting!)
enter this script in package.json: “scripts”: { “test”: “mocha --exit” }
Thanks for the
--exit
. It fixed it for me.…and here are the docs
Yay! Turns out wtfnode is much better at shedding light on this. I found out that when using node-fetch, I have to call
result.text()
or.json()
or the connection stays open. Runningwtfnode ./node_modules/.bin/_mocha
and pressing <kbd>Ctrl-C</kbd> revealed the open connections.I think it would have been easier if the docs recommended this package rather than
why-is-node-running
.Another suggestion: I really like @andywer’s idea of having some kind of warning if mocha dosn’t exit. It would be sweet if mocha could even use
wtfnode
somehow to list active handles keeping node alive after some time has passed.Whatever the right thing should be, I think it’s garbage that I had to look this far to figure out why mocha wouldn’t exit. Anyway, I’ve given my suggestions 😃
If you’re still having problems and
why-is-node-running
doesn’t work right, check out wtfnode.That package worked much better for me.
Based on the documentation @pgilad sent here, to me there is a cleaner solution. As the documentaion says:
A cleaner solution then it would be to create a global
after
function (anafter
outside anydescribe
function), I would recommend in a separate file likeexit-mocha.js
orexit-mocha
if you will. The callback sent to after you can force a gracefully node process exit which is going to exit without any error. The file can be sent tomocha
cli as if it was another test file (it could simulate an--exit
flag)exit-mocha.js
orexit-mocha
Then you can execute mocha tests like:
mocha exit-mocha test/**/*.spec.js
or
mocha exit-mocha.js test/**/*.spec.js
It is important that if you are using wildcards for the name of the tests files like I did with
test/**/*.spec.js
that the name of theexit-mocha
file does NOT match the wildcard pattern, otherwise, it will not be possible for you to use it as a “flag”Created mochajs/mochajs.github.io#81 and #3045 to track updating the documentation and Mocha’s Node flags, respectively. Will keep this issue open at least till the documentation is updated.
Right, sorry I wasn’t clear about that –
--expose-internals
is a Node option that can be used by runningnode --expose-internals ./node_modules/mocha/bin/_mocha
instead of./node_modules/.bin/mocha
. That’s also something we can fix, since Mocha passes certain flags on to Node and we can add--expose-internals
to those flags.@mjgs it did for me -
wtfnode
worked but only when I used it with the_mocha
binary, not themocha
one:Off-topic: I love this ticket, just for the information on troubleshooting tools it keeps on giving! 😃
Hi guys, I think this feature might still be not working properly, this is the most basic test I could think of and mocha still fails to exit by itself.
--no-exit
is an excellent default option and I am all for it, but either I’m failing at understanding what I am doing wrong or something is intrinsically wrong with mocha and it’s preventing to close even most simple of tests.describe('describe', function() { it('it', function(done) { done(); }); });
summary:
--exit
does work,--no-exit
never closes any test.would recommend the new node inspector’s debugger… it has good async traces.
@DanielSmedegaardBuus TL;DR on the solution: put
--exit
in themocha.opts
file. (This is typically the solution for anything that needs to always be set when Mocha is run, as a more general tip.) More detailed clarification of what this change is about: below.#2640 is about promise rejections, isn’t intended to do anything but make them consistent with synchronous exceptions thrown from non-promise asynchronous test code, and hasn’t been implemented yet.
This is about tests setting up resources, listeners or ongoing work and not cleaning it up, regardless of whether promises are used in their implementation and/or in the test. For example:
I have a test for an API based on websockets, which includes opening and closing connections. This API was buggy and was not closing the connections right, but passed my tests since the tests didn’t really have a way to assert that the underlying connection was treated correctly. (If I had strictly tested only my own code, I could have verified it’s using the dependency in what I mistakenly thought was the right way; I was testing with the real websocket precisely to catch issues with usage correctness in the first place however – integration tests to complement the unit tests.) I caught that error when I switched to the
--no-exit
behavior (of Mocha 3; now the default in Mocha 4) and discovered that if I ran those tests Node would not close because the websocket connection is still listening.(It’s true it’s possible that the error lies in a dependency rather than in your own code, but even then, there can be value in knowing that before you ship with a given version of that dependency – as alluded-to above, this is more applicable for integration tests in the first place than for unit tests, but ideally a project has both of those.)
This isn’t always necessary, of course – in some cases, a resource of this nature might be meant to be left alive till the process is killed, or might keep Node alive even though no other cleanup of the resource really matters, or might be part of a resource pool that reuses individual members and doesn’t need to clean up the pool as a whole – in such cases
--exit
would be correct. The reason for the change of the default behavior was to raise the visibility of such issues: before, you would probably never know to try--no-exit
and check for these kinds of errors, now you’ll run into them by default and can--exit
if you determine that the behavior is correct (or at least not worth worrying about).There is, of course, something of an issue with the “fix” that just hanging when this happens isn’t very informative. We’ve still got to figure out if we can integrate some way of actually programmatically checking for things still running (so a proper warning or error can be given) that will work on all supported versions of Node without interfering with anything that might still be cleaning up or closing when Mocha reaches the end.
I must admit that I don’t quite understand the issues in the referenced tickets that lay behind this new behavioural change.
AFAICT, it has something to do with unhandled Promise rejections. But pardon me, if you don’t resolve a Promise given to a Mocha test, then Mocha will neither succeed nor continue (if
--bail
is set) from that test, right? So it must be some poorly implemented nested Promise which doesn’t have a rejection handler, but the containing code must be resolving anyway.If this is the case, I don’t see how it’s Mocha’s job to detect those issues. And if implementing some magic to detect those issues, which may make current (correctly-written) tests hang indefinitely, then that magic should be an opt-in behaviour, not an opt-out one — i.e.
--no-exit
rather than--exit
.I’m looking at all of my tests using the official node-mongodb-native driver hanging because that driver pools connections and don’t close them all on
.close()
, seemingly be design. My tests are behaving properly, but a dependency is not (or is it? I don’t know necessarily for a fact that it’s bad behaviour to have lingering sockets or file descriptors until a script is exited elsewhere — do you?). So what am I testing here? My code, or someone else’s? I thought I was testing my own, but apparently not.Sure, I can add the
--exit
flag, but the next guy might not, and either way, it just seems wrong that I can write perfectly fine tests with perfectly fine code being tested, and still have some random dependency cause my tests to hang indefinitely.If I add a
process.exit()
in a finalafter()
hook, my tests won’t hang, but then it’ll break horribly with--watch
(possibly other features), not only preventing me from watching for changes, but ejecting me into a cursorless shell.It may very well be that I’m the clueless one here (certainly there’s a lot I’m not getting, and a lot of people have been involved, so I would tend to think so 😃 ), I just feel like this whole thing is not quite… right…?
Cheers 😃
EDIT: Given this behavior, is there any way for me to check inside a Mocha test if it’s being run with
--watch
so that I can make sure not to callprocess.exit()
and break things?I can see how having existing tests suddenly stop exiting can be disconcerting (to say the least). I do see a mention about the new behavior in the release notes.
I discovered the change accidentally myself with new tests that forced me to ensure I called redis.quit() in an after() function and I am definitely pleased with the new behavior, which seems correct and appropriate to me.
I didn’t need to use [this gist] this time, but as already mentioned by @boneskull, it looks like it could be really useful when it’s not clear which async tasks haven’t completed and are holding up the process from exiting.
Sorry to comment on the closed PR again, but there might a user-friendly solution here:
One could log a warning about the changed behavior after the runner has finished for 3s or so, but the process didn’t exit. Would just need to add a
setTimeout()
after the runner finished and call timeout.unref() to make sure this timeout doesn’t prevent the process from exiting. If the timeout gets executed it’s time for a warning 😉Here’s a Gist with a rough guide to work around this for the time being.
@ScottFreeCode
Edit:
This works:
"mocha --reporter mocha-allure-reporter ./tests/controllers --exit"
worked for me. Indeed the--exit
is a very good workaround. I use the version 5.2.0 in my project.It just seems to me that this is about making more noise, not signal: in the significant majority of cases no one’s app is going to get better because Mocha hung at the end.
If Mocha is going to default, it seems like it should default to ending when tests (and all
after
/afterEach
calls) have finished. Not doing so just to tell people “hey your artificial testing environment is artificial” doesn’t benefit the majority (or even a decent minority) of users.If people really want to debug unclosed connections then it seems that should be the case you provide an option for. But all the rest of the time, does it really benefit the library’s users to confuse everyone else with (what amounts to saying) “you’re in a testing environment and one of a billion possible things is open”?
To put it another way, if you’re going to tell 99% of the people that run into this “just use
--exit
” then maybe--exit
shouldn’t be a special option you have to provide … maybe the default behavior should be to serve 99% of the user cases (while of course still giving users the option of--tell-me-if-I-have-unclosed-stuff-in-my-testing-environment-and-that-is-actually-what-i-am-trying-to-find-out
)?I mean, if you made that the option, realistically how often do you think people would even pass it?
If wtfnode indicates that a mongo db connection is open, then it’s open. Why would you assume it’s wrong?
@borisovg Imagine how great this ticket would be if it actually provided a solution! 😃 @boneskull Is there some “list File descriptors/Sockets/Servers/Timer” debugger feature that I am not aware of?
That’s what a debugger’s for, IMO. How would you debug your own scripts if they never exited?
https://github.com/GoogleChromeLabs/ndb is a cool project.
In my case I’m using a Koa application, and testing with Mocha 4 + Supertest.
I had to just close server together with
done
call following release notes “To avoid false positives and encourage better testing practices”.Before:
After:
Hope it helps somenoe.