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

When using bail flag, before hook failures do not exit as failed #3303

Closed
4 tasks done
marcusmotill opened this issue Apr 3, 2018 · 2 comments · Fixed by #3346
Closed
4 tasks done

When using bail flag, before hook failures do not exit as failed #3303

marcusmotill opened this issue Apr 3, 2018 · 2 comments · Fixed by #3346
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@marcusmotill
Copy link

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq 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

When using the bail flag before hook failures do not exit the test as a failure.

Steps to Reproduce

sample test:

//index.js
const assert = require('assert');

describe('some test', () => {
    before(async () => {
        throw new Error('should die');
    });

    it('should not get here', () => {
        assert(false);
    });
});

command:

mocha --bail index.js

output:

  some test

  0 passing (6ms)

    1) "before all" hook

Although its hard to see here, the test is exiting with a 0 code. You can see it better by running:

mocha --bail index.js && echo 'hello'

  some test

  0 passing (9ms)

    1) "before all" hook

hello

The echo 'hello' should never get hit but it does.

Similarly, running this without --bail works as expected, the process exits with a non-zero code:

mocha index.js && echo 'hello'

output:

  some test
    1) "before all" hook


  0 passing (7ms)
  1 failing

  1) some test
       "before all" hook:
     Error: should die
      at Context.<anonymous> (index.js:5:15)

Note: Regardless of before block structure this bug still occurs

const assert = require('assert');

describe('some test', function() {
    before(function() {
        throw new Error('should die');
    });

    it('should not get here', function() {
        assert(false);
    });
});
// still bad

Expected behavior: Test should fail if there is an error in before hook

Actual behavior: Test does not fail

Reproduces how often: 100%

Versions

mocha v5.0.5 global and local (tested version 4 through 5 and 5.0.5 seems to be the only affected version)
macOS
zsh

Additional Information

Related to: #3096

@outsideris outsideris added type: bug a defect, confirmed by a maintainer status: accepting prs Mocha can use your help with this one! labels Apr 4, 2018
mattwagl pushed a commit to thenativeweb/roboter that referenced this issue Apr 17, 2018
Test case to validate this mocha bug mochajs/mocha#3303.
@yjhuoh
Copy link

yjhuoh commented Apr 17, 2018

can confirm this issue is real. we ended up checking in a bunch of failing code because of it. this is the change that caused it:

#3278

@sgress454
Copy link

FYI, issue affects "after" hooks as well.

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.

4 participants