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) and mocha --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

Most upvoted comments

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 upgrade

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, but why-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:

"scripts": {
    "test": "mocha --exit"
}

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:

  • run Mocha with --expose-internals (node --expose-internals node_modules/mocha/bin/_mocha)
  • make your first test file (e.g. listed in 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.

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. Running wtfnode ./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:

To avoid false positives and encourage better testing practices, Mocha will no longer automatically kill itself via process.exit() when it thinks it should be done running.

A cleaner solution then it would be to create a global after function (an after outside any describe function), I would recommend in a separate file like exit-mocha.js or exit-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 to mocha cli as if it was another test file (it could simulate an --exit flag)

exit-mocha.js or exit-mocha

after('Exit mocha gracefully after finishing all tests execution'. function () {
  // Exit node process
  process.exit();
});

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 the exit-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 running node --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 the mocha one:

$ wtfnode ./node_modules/.bin/_mocha my-shitty-code.spec.js 


  tests
    ✓ some test


  1 passing (5ms)

^C[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
- Servers:
  - :::8080 (HTTP)
    - Listeners:
      - request: (anonymous) @ /home/borisov/test/my-shitty-code.js:4
- Intervals:
  - (5000 ~ 5 s) (anonymous) @ /home/borisov/test/my-shitty-code.js:8

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 the mocha.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 final after() 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 call process.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 😉

@ScottFreeCode

borisov@glossy:~/test/mocha $ node --version
v6.11.3

borisov@glossy:~/test/mocha $ ./node_modules/.bin/mocha --version
4.0.0

borisov@glossy:~/test/mocha $ ./node_modules/.bin/mocha --expose-internals test.spec.js 
  error: unknown option `--expose-internals'

Edit:

This works:

node --expose-internals ./node_modules/.bin/mocha test.spec.js

"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:

request(app.listen())
  .post('/')
  .send(requestSrc)
  .expect({ f: {} }, done)

After:

const server = app.listen()

request(server)
  .post('/')
  .send(requestSrc)
  .expect({ f: {} }, () => {
    server.close()
    done()
  })

Hope it helps somenoe.