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 when current test is pending #4083

Merged
merged 3 commits into from Nov 22, 2019

Conversation

juergba
Copy link
Member

@juergba juergba commented Oct 25, 2019

Description

describe('test', () => {
  it('test1', () => {
    setTimeout(() => {
      throw new Error('Uncaught');
    }, 3);
  });
  it('test2', function () { this.skip(); });
  it('test3', function () { this.skip(); });
  it('test4', function () { this.skip(); });
});

output with exitCode: 0

test suite
    √ test1
    - test2
    - test3
    - test4

  1 passing (34ms)
  3 pending

Description of the Change

Runner#uncaught:

  • per default Mocha tries to map uncaughtException's to the correct test case, to recover and keep the runner working. This is quite simple with synchronous tests, in an async scenario (by design or by coder's mistake) it's done on a "best effort" basis.
    Currently the user has no possibility to escape this recovery effort, not even with the --allow-uncaught option. Now --allow-uncaught throws an error and exit Mocha's process.

  • current test is pending: when an uncaughtException bubbles up, we retrospectively label the pending test as failed, then keep the runner going. That way the exitCode will be >0.

When an uncaughtException bubbles up we can't clearly determine its origin. I suppose our recovery process is responsible for a few weird bugs, eg. number of executed tests is varying when rerun, errors are swallowed (reporters?) or non-existing hooks throw errors. So its important to have an effective mean to throw and exit when tracing fuzzy errors.

Recovery process, when current test is:

  • still open: report test as failed, "best effort" recovery, runner keeps going
  • failed: ignore uncaughtException, runner keeps going
  • pending: report test retrospectively as failed, runner keeps going << new
  • passed with success: report test as failed, abort runner

Applicable issues

closes #3938

@coveralls
Copy link

coveralls commented Oct 25, 2019

Coverage Status

Coverage decreased (-0.02%) to 92.723% when pulling a0b203d on juergba/uncaught-swallow into b9fbd69 on master.

@juergba juergba force-pushed the juergba/uncaught-swallow branch 2 times, most recently from 5e40b1c to 75b95bd Compare November 4, 2019 10:04
@juergba juergba changed the title uncaughtException: fix ignoring them when test has already failed or is pending uncaughtException: fix when current test is pending Nov 4, 2019
@juergba juergba marked this pull request as ready for review November 4, 2019 18:01
@juergba juergba self-assigned this Nov 4, 2019
@juergba juergba added area: async related to asynchronous use of Mocha type: bug a defect, confirmed by a maintainer status: needs review a maintainer should (re-)review this pull request semver-major implementation requires increase of "major" version number; "breaking changes" labels Nov 4, 2019
@juergba juergba added this to the v7.0.0 milestone Nov 4, 2019
Copy link
Contributor

@craigtaub craigtaub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@juergba juergba merged commit 46ca9ac into master Nov 22, 2019
@juergba juergba deleted the juergba/uncaught-swallow branch November 22, 2019 16:12
@juergba juergba removed the status: needs review a maintainer should (re-)review this pull request label Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: async related to asynchronous use of Mocha semver-major implementation requires increase of "major" version number; "breaking changes" type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncaught exceptions are silenced if happen in timeframe of skipped test
3 participants