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

Ignore asynchronous failures after spec resolution #1578

Merged
merged 2 commits into from
Mar 6, 2015
Merged

Ignore asynchronous failures after spec resolution #1578

merged 2 commits into from
Mar 6, 2015

Conversation

danielstjules
Copy link
Contributor

This improves on #1540 From that thread:

Given the following test, which only has one spec:

it('fails exactly once when a global error is thrown first', function(done) {
  setTimeout(function() {
    throw new Error('global error');

    setTimeout(function() {
      done(new Error('test error'));
    }, 0);
  }, 0);
});

You get two two failures:

$ ./bin/mocha pr1540.js

  ․․

  0 passing (6ms)
  2 failing

  1)  fails exactly once:
     Error: test error
      at null._onTimeout (/Users/danielstjules/git/mocha/pr1540.js:10:12)
      at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
      at timers.js:117:18
      at process._tickCallback (node.js:415:13)
      at process._tickFromSpinner (node.js:390:15)

  2)  fails exactly once:
     Error: test error
      at null._onTimeout (/Users/danielstjules/git/mocha/pr1540.js:10:12)
      at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
      at timers.js:117:18
      at process._tickCallback (node.js:415:13)
      at process._tickFromSpinner (node.js:390:15)

But after this PR, it correctly only shows one failure, and highlights the error thrown first ("global error"):

$ ./bin/mocha pr1540.js

  

  0 passing (6ms)
  1 failing

  1)  fails exactly once:
     Uncaught Error: global error
      at null._onTimeout (/Users/danielstjules/git/mocha/pr1540.js:12:11)
      at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

In addition to the above fix, this PR improves on #1540 by also correctly handling the following test case:

it('fails exactly once when a global error is thrown second', function(done) {
  setTimeout(function() {
    done(new Error('test error'));
  }, 0);

  setTimeout(function() {
    throw new Error('global error');
  }, 0);
});

It now also only reports a single failing spec, rather than two.

jugglinmike and others added 2 commits February 7, 2015 19:12
`Runner#uncaught` labels the currently-executing `Runnable` instance as
"failed" in response to uncaught exceptions. If the the `Runnable`
instance later signals failure through Mocha's asynchronous interface,
the `Runner` counts this as a distinct test failure. Consequently,
reporters generate inaccurate reports (and in some cases, this triggers
uncaught exceptions causing the process itself to crash).

When a runnabe completes, explicitly check that it has not already been
labeled as failing before reporting any additional errors.
@travisjeffery
Copy link
Contributor

Awesome! Thanks

travisjeffery pushed a commit that referenced this pull request Mar 6, 2015
Ignore asynchronous failures after spec resolution
@travisjeffery travisjeffery merged commit 49d81aa into mochajs:master Mar 6, 2015
@danielstjules
Copy link
Contributor Author

No problem! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants