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

multiple calls to done() doesn't always fail if second done is called async #4151

Closed
boneskull opened this issue Jan 13, 2020 · 2 comments
Closed
Assignees
Labels
area: async related to asynchronous use of Mocha type: bug a defect, confirmed by a maintainer

Comments

@boneskull
Copy link
Member

Example:

it('should fail', function (done) {
  process.nextTick(function () {
    done();
    setTimeout(done);
  });
});

The above won't fail with a nonzero exit code. EVENT_TEST_FAIL is emitted, but nothing is listening at this point.

To address this:

  1. The Runner must have a notion of a "stopped" state
  2. If a test fails while the Runner is in the stopped state, then it must throw an exception.
  3. How this exception is handled depends on whether the standard uncaught exception handler is still bound. If it is, we let it be handled, and we get a proper stack trace back to the test.
  4. If the standard uncaught exception handled has been disabled (which happens when EVENT_RUN_END is emitted)--and the process will fail with a bog-standard uncaught exception--Mocha will need to retain extra information in the error so the user can find which test is responsible. I'm not sure why we lose the stack trace back to the test; maybe somebody can help.

Anyway, I have a fix in-progress for this.

Ref: https://twitter.com/jsumners79/status/1216391159699988481

@boneskull boneskull added type: bug a defect, confirmed by a maintainer area: async related to asynchronous use of Mocha labels Jan 13, 2020
@boneskull boneskull self-assigned this Jan 13, 2020
boneskull added a commit that referenced this issue Jan 14, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
boneskull added a commit that referenced this issue Jan 14, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
boneskull added a commit that referenced this issue Jan 14, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
- force color in Travis CI b/c my eyes
boneskull added a commit that referenced this issue May 13, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
- force color in Travis CI b/c my eyes
boneskull added a commit that referenced this issue May 14, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
- force color in Travis CI b/c my eyes
boneskull added a commit that referenced this issue May 19, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
- force color in Travis CI b/c my eyes
boneskull added a commit that referenced this issue May 19, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
- force color in Travis CI b/c my eyes
craigtaub pushed a commit that referenced this issue May 21, 2020
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called when a test fails twice by other means (unsure what those means are yet)
- force color in Travis CI b/c my eyes
- remove `Runner#uncaughtEnd`; move logic to `Runner#uncaught`, since we can now rely on the value of `Runner#state`.
- upgrade `unexpected-eventemitter`
- fix potential listener leak in `Runner#run`
@iqianxing
Copy link

iqianxing commented Jun 7, 2021

async test throw an uncaught error when test is timeout.
my test code like below:

var assert = require('chai').assert;
describe("# async test", function () {
  this.timeout(2000);

  it("async error", function (done) {
    var errorMessage = "some error"
    setTimeout(function () {
      assert.notOk(errorMessage)
      done();
    }, 5000)
  })
})

i run my test , an uncaught error display on my console like below:
image

error detail:

mocha\lib\runner.js:962
    throw err;

image

@iqianxing
Copy link

async test throw an uncaught error when test is timeout.
my test code like below:

var assert = require('chai').assert;
describe("# async test", function () {
  this.timeout(2000);

  it("async error", function (done) {
    var errorMessage = "some error"
    setTimeout(function () {
      assert.notOk(errorMessage)
      done();
    }, 5000)
  })
})

i run my test , an uncaught error display on my console like below:
image

error detail:

mocha\lib\runner.js:962
    throw err;

image

some async code is still running after async test task is stopped.

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 type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

2 participants