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

Do not force exit of a process (by default) #2879

Closed
medikoo opened this issue Jun 12, 2017 · 11 comments
Closed

Do not force exit of a process (by default) #2879

medikoo opened this issue Jun 12, 2017 · 11 comments
Labels
semver-major implementation requires increase of "major" version number; "breaking changes" status: accepting prs Mocha can use your help with this one!

Comments

@medikoo
Copy link

medikoo commented Jun 12, 2017

I'm talking about behaviour that can be switched off with --no-exit option.

Having this as default looks dangerous, as it may hide serious issues in tests (any orphaned async flow is just silenced, leaving no clue to developer)

e.g. on my side I had a promise with crashing test not returned properly. Still test process as handled by mocha was exiting smiling with a success. It took me a while to figure out why it happens.

@michaelBenin
Copy link

@medikoo did you try having a call to done() in the test and increase the timeout?

@medikoo
Copy link
Author

medikoo commented Jun 12, 2017

@michaelBenin no, it was a setup issue, it was orphaned async flow not provided to mocha as documented.

So describe block ended with no test being run, while there were scheduled ones and what's worse, the scheduled ones were failing.

But fact that API was misused should be irrelevant, what's relevant is that background async tasks (which in the end would crash) where just aborted cause of forced process exit. Especially test suites should not do this.

@michaelBenin
Copy link

In my tests I have a teardown util which is the last test to be run. Would something like this benefit you?

@medikoo
Copy link
Author

medikoo commented Jun 13, 2017

@michaelBenin this issue is about silencing eventual errors in configuration which in normal cricumstances will explicitly crash (and that way become apparent).

Otherwise once spotted, those errors are easy to fix, there's no problem with that.

@ScottFreeCode ScottFreeCode added status: accepting prs Mocha can use your help with this one! semver-major implementation requires increase of "major" version number; "breaking changes" labels Jun 13, 2017
@ScottFreeCode
Copy link
Contributor

See also #1855 for a potential caveat about eventually timing out so this doesn't lead to just sitting waiting on anything that keeps the process open forever.

@medikoo
Copy link
Author

medikoo commented Jun 13, 2017

so this doesn't lead to just sitting waiting on anything that keeps the process open forever.

In my opinion tests always should come up with proper cleanup. If effect of a test is that process is kept forever then it is a clear issue with a test or a functionality that's being tested. I believe it's quite important that such issues are exposed to the developer

@ScottFreeCode
Copy link
Contributor

If we want to get this in V4, it should be a simple matter of switching the default for the parameter... right?

ScottFreeCode added a commit that referenced this issue Sep 29, 2017
Allows `--exit` to override and force exit as before. Resolves #2879
ScottFreeCode added a commit that referenced this issue Sep 29, 2017
Allows `--exit` to override and force exit as before. Resolves #2879
boneskull pushed a commit that referenced this issue Sep 30, 2017
Allows `--exit` to override and force exit as before. Resolves #2879
boneskull pushed a commit that referenced this issue Oct 1, 2017
Allows `--exit` to override and force exit as before. Resolves #2879
paulmelnikow pushed a commit to IcedFrisby/IcedFrisby that referenced this issue Oct 19, 2017
@maximilianschmitt
Copy link

Hi, we're looking to update to mocha 4 and think this is a great change.

However, is there a way to tell when mocha intends to exit / all tests are done? We have some setup-code that spawns a server that is used across multiple test suites (so before(), after() don't work for that). Thanks!

@boneskull
Copy link
Contributor

@maximilianschmitt I would suggest opening a server in a separate process

@ScottFreeCode
Copy link
Contributor

Global ("root-level") before and after, i.e. the ones outside any describe, should run at the very beginning and very end of Mocha's test run.

@maximilianschmitt
Copy link

Thanks @ScottFreeCode , that should do it! :)

sgilroy pushed a commit to TwineHealth/mocha that referenced this issue Feb 27, 2019
Allows `--exit` to override and force exit as before. Resolves mochajs#2879
Gasol added a commit to Gasol/aglio that referenced this issue May 13, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
Gasol added a commit to danielgtaylor/aglio that referenced this issue May 13, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
Gasol added a commit to Gasol/aglio that referenced this issue May 13, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
Gasol added a commit to Gasol/aglio that referenced this issue May 13, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
Gasol added a commit to Gasol/aglio that referenced this issue May 13, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
Gasol added a commit to Gasol/aglio that referenced this issue May 14, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
Gasol added a commit to Gasol/aglio that referenced this issue May 14, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
Gasol added a commit to Gasol/aglio that referenced this issue May 15, 2019
Use NPM scripts to run coffeelint

- Update coffeelint to 2.1.0

  Also update Coffeescript and explictly depends on version 2,
  Use `undefined` as default argument instead of null, it's breaking
  changes on Coffescript 2.

  See:
    - https://coffeescript.org/#breaking-changes-default-values
    - jashkenas/coffeescript#2201

- Remove grunt and its plugins

  inlcude grunt-coffeelint, grunt-contrib-coffee and grunt-mocha-cov,
  Remove Gruntfile.coffee by using NPM scripts, and remove path of "npm bin"
  in scripts, Because the command of "npm run" adds node_modules/.bin to the
  PATH provided to scripts automatically.

- Update vulnerable dependencies

  include socket.io, chokidar

- Explictly depends on mocha

  Add --exit flag

  "By default, Mocha will no longer force the process to exit once all
  tests complete. This means any test code (or code under test) which
  would normally prevent node from exiting will do so when run in Mocha.
  Supply the --exit flag to revert to pre-v4.0.0 behavior"

  mochajs/mocha#2879
  https://mochajs.org/#-exit

- Use c8 to output coverage instead of blanket
- Add generate-examples scripts

  To replace 'gen-examples' task defined in Gruntfile
bot-linagora pushed a commit to linagora/linagora.esn.chat that referenced this issue Aug 27, 2019
…chacli options

To force shutdown of the event loop after test run

From v4.0.0, grunt-mocha-cli use mocha v4.0.0 which no logger use '--no-exit' flag

By default, Mocha will no longer force the process to exit once all tests complete. This means any test code (or code under test) which would normally prevent node from exiting will do so when run in Mocha. Supply the --exit flag to revert to pre-v4.0.0 behavior

Check breaking change: https://github.com/mochajs/mocha/releases/tag/v4.0.0

The reported Github issue: mochajs/mocha#2879
kanedafromparis added a commit to kanedafromparis/nodejs-ex that referenced this issue Dec 10, 2019
this is a mochas issue .
mochajs/mocha#2879
This might be better on the long run to simply update dependency version
jonathan-briere added a commit to jonathan-briere/swapi-graphql that referenced this issue Jul 4, 2023
darkhorse941019 pushed a commit to darkhorse941019/express-graphql that referenced this issue Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major implementation requires increase of "major" version number; "breaking changes" status: accepting prs Mocha can use your help with this one!
Projects
None yet
Development

No branches or pull requests

5 participants