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

uncaughtException: fix double EVENT_RUN_END events #4025

Merged
merged 2 commits into from
Sep 26, 2019

Conversation

juergba
Copy link
Contributor

@juergba juergba commented Sep 20, 2019

Description

Per default Mocha tries to recover from uncaughtException and to map the exception to the correct test case. It's not possible to recover gracefully if a Runnable has already passed successfully and then fails asynchronously.

Firing an EVENT_RUN_END out of a suite other than the root suite is wrong and will result in the emission of double EVENT_RUN_END events. The first event will not abort the runner and a second event will be fired by the root suite. We already had a similar issue, see #3617.

To prevent double test epilogues the reporters have been "fixed" in the past by using runner.once(EVENT_RUN_END, ...).

Following tests were ineffective:

  • test\integration\fixtures\uncaught-fatal.fixture.js:
√ should bail if a successful test asynchronously fails
  1) should bail if a successful test asynchronously fails

  1 passing (8ms)
  1 failing

  1) should bail if a successful test asynchronously fails:
     Uncaught Error: global error
      [...]

  √ should not actually get run      // runner keeps going after first end event
  • test\integration\fixtures\regression\issue-1327.fixture.js
testbody1
  √ test 1
  1) test 1

  1 passing (8ms)
  1 failing

  1) test 1:
     Uncaught Error: Too bad
      [...]

testbody2              // runner keeps going after first end event
  √ test 2
testbody3
  2) test 3

Description of the Change

Instead of emitting an EVENT_RUN_END event we use Suite#bail() Runner#abort() to bail the runner cleanly.

Applicable issues

@coveralls
Copy link

coveralls commented Sep 20, 2019

Coverage Status

Coverage increased (+0.07%) to 92.734% when pulling 1fbb9f0 on juergba/uncaught-bail into eed38d7 on master.

@juergba juergba self-assigned this Sep 20, 2019
@juergba juergba added type: bug a defect, confirmed by a maintainer status: needs review a maintainer should (re-)review this pull request semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Sep 20, 2019
@juergba juergba marked this pull request as ready for review September 20, 2019 08:02
@juergba
Copy link
Contributor Author

juergba commented Sep 24, 2019

@outsideris I had to adapt my former solution.

Bailing the current suite bailed only the current suite, nevertheless subsequent suites were still executed.
Aborting the runner bails the runner completely. Runner#abort() is used also in our watch feature to abort the runner before restarting a new test run.

@juergba juergba merged commit 5f8df08 into master Sep 26, 2019
@juergba juergba deleted the juergba/uncaught-bail branch September 26, 2019 05:54
@juergba juergba removed the status: needs review a maintainer should (re-)review this pull request label Sep 26, 2019
@juergba juergba added this to the next milestone Sep 26, 2019
@juergba juergba modified the milestones: 6.2.1, 6.2.2 Oct 18, 2019
boneskull pushed a commit that referenced this pull request Oct 18, 2019
@boneskull boneskull added the landed-on-v6.2.x cherry-picked on v6.2.x branch from master label Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
landed-on-v6.2.x cherry-picked on v6.2.x branch from master semver-patch implementation requires increase of "patch" version number; "bug fixes" type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants