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

Bail with failing after() runs suite twice #3096

Closed
4 tasks done
rawb opened this issue Nov 6, 2017 · 3 comments · Fixed by #3278
Closed
4 tasks done

Bail with failing after() runs suite twice #3096

rawb opened this issue Nov 6, 2017 · 3 comments · Fixed by #3278
Assignees
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@rawb
Copy link

rawb commented Nov 6, 2017

Prerequisites

  • Checked that your issue isn't already filed by cross referencing issues with the common mistake label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with:
    node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend avoiding the use of globally installed Mocha.

Description

With a failing test suite using the bail option in conjuncture with an after clause that throws an error.
Mocha bails properly, executes the after as expected.
Then it does it all again. But Why?

Steps to Reproduce

Very simple repro steps.
File new with latest mocha.

"scripts": {
    "test": "./node_modules/.bin/mocha --reporter spec --bail"
  }

and

describe("some suite", function() {
    it("does the first test", function() {
        throw new Error('first error')
    });
    after(() => {
        throw new Error('after error')
    })
})

yarn test, observe double suite run
remove error in after, observe one suite run

Expected behavior: [What you expect to happen]
test suite is only ran once
Actual behavior: [What actually happens]
test suite is ran twice
Reproduces how often: [What percentage of the time does it reproduce?]
100%

Versions

node node_modules/.bin/mocha --version => 4.0.1
macOS Sierra 10.12.6
zsh

@ScottFreeCode ScottFreeCode added the type: bug a defect, confirmed by a maintainer label Nov 7, 2017
@ScottFreeCode
Copy link
Contributor

Seems to happen for me no matter which hook is used (after, before, afterEach, beforeEach), regardless of whether the test passes or fails, and no matter whether they are inside a describe or not.

Doesn't rerun the tests as far as I can tell, just prints the final summary/results twice -- probably some variation of #2906 (strange that there are so many different things that can cause such behavior).

@prop
Copy link

prop commented Mar 29, 2018

After this fix Mocha exits incorrectly if hook throws an error:

$ cat app/test/hook.js
describe('suite', function(){
  before(() => {
    throw Error('Hook error');
  });

  it('test', function(){

  });
})
$ ./node_modules/.bin/mocha -b app/test/hook.js


  suite

  0 passing (7ms)

    1) "before all" hook

$ echo $?
0

Non-zero exit status expected.

This was referenced Sep 22, 2018
@plroebuck
Copy link
Contributor

plroebuck commented Oct 19, 2018

Possible that the fix to this problem should have been done here instead?

Done is done, right? Just process the event once.

this.once('end', function () {
    debug('end');
    process.removeListener('uncaughtException', uncaught);
    fn(self.failures);
});

If proposed change messes up the expected event chain (i.e., 'test end', 'suite end' never trigger), we have a major problem. Do we need a harakiri event?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants