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

Improve hook pattern of 'this.skip()' in beforeAll hooks #4223

Merged
merged 2 commits into from Apr 18, 2020

Conversation

juergba
Copy link
Member

@juergba juergba commented Apr 6, 2020

Description

describe('outer suite', function() {
  before(function() {
    console.log('** before-1');
    this.skip();
  });
  before(function() { console.log('** before-2'); });  // second before hook should not run

  it('outer pending test', function() { });

  describe('inner suite', function() {
    before(function(done) {              // inner async before hook should not run
      console.log('**** before: done');
      done();
    });

    it('inner pending test', function() { });

    after(function(done) {               // inner async after hook should not run
      console.log('**** after: done');
      done();
    });    
  });

  after(function() { console.log('** after'); });
});
  outer suite
** before-1
** before-2                  // second before hook should not run
    - outer pending test
    inner suite
**** before: done            // inner async before hook should not run
      - inner pending test
**** after: done             // inner async after hook should not run
** after

  0 passing (16ms)
  2 pending

Description of the Change

  • since this.skip() aborts hook execution, subsequent before hooks on the same suite level are not executed anymore.
  • inner before/after hooks: explicit async hooks with done argument are not executed anymore.
    Note: sync or promise-returning hooks work correctly, no changes are required.
  • test.state: is set to pending.
    We have three state's now: failed, passed, pending

Applicable issues

related #3225

@coveralls
Copy link

coveralls commented Apr 7, 2020

Coverage Status

Coverage increased (+0.02%) to 92.818% when pulling 48bb3be on juergba/pending into 54475eb on master.

@juergba juergba requested a review from a team April 8, 2020 08:05
@juergba juergba self-assigned this Apr 8, 2020
@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-patch implementation requires increase of "patch" version number; "bug fixes" labels Apr 8, 2020
@juergba juergba marked this pull request as ready for review April 8, 2020 08:07
@juergba juergba merged commit 8236ffd into master Apr 18, 2020
@juergba juergba added semver-major implementation requires increase of "major" version number; "breaking changes" and removed status: needs review a maintainer should (re-)review this pull request semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Apr 18, 2020
@juergba juergba added this to the v8.0.0 milestone Apr 18, 2020
@juergba juergba deleted the juergba/pending branch April 18, 2020 06:13
@teddibaer
Copy link

It seems to me, that the current behavior of mocha regarding calling this.skip inside of before() hooks is not correctly reflected in the documentation, where it states that:

"before/after hooks are skipped unless they are defined at the same level as the hook containing this.skip()"

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.

None yet

3 participants