From 9c83f23416ecdba8f6a667f17d39acf1519e262d Mon Sep 17 00:00:00 2001 From: Juerg B <44573692+juergba@users.noreply.github.com> Date: Wed, 19 Dec 2018 18:51:30 +0100 Subject: [PATCH] Behavior of after/afterEach hooks with --bail flag (#3617) * runner.js: delete second end emit * tests * documentation * runner.js * tests: corrections closes #3398, closes #3598, closes #3457, closes #3617 --- docs/index.md | 2 +- lib/runner.js | 8 +- .../fixtures/options/bail-async.fixture.js | 23 +++++ .../options/bail-with-after.fixture.js | 54 +++++++++++- .../options/bail-with-afterEach.fixture.js | 57 ++++++++++++ .../options/bail-with-before.fixture.js | 41 ++++++++- .../options/bail-with-beforeEach.fixture.js | 46 ++++++++++ .../options/bail-with-test.fixture.js | 47 ++++++++++ .../fixtures/options/bail.fixture.js | 8 +- test/integration/options.spec.js | 88 +++++++++++++++++-- test/unit/runner.spec.js | 8 -- 11 files changed, 351 insertions(+), 31 deletions(-) create mode 100644 test/integration/fixtures/options/bail-async.fixture.js create mode 100644 test/integration/fixtures/options/bail-with-afterEach.fixture.js create mode 100644 test/integration/fixtures/options/bail-with-beforeEach.fixture.js create mode 100644 test/integration/fixtures/options/bail-with-test.fixture.js diff --git a/docs/index.md b/docs/index.md index d07b8fc5b7..d8a76a8a8d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -909,7 +909,7 @@ Enforce a rule that tests must be written in "async" style, meaning each test pr ### `--bail, -b` -Causes Mocha to stop running tests after the first test failure it encounters. +Causes Mocha to stop running tests after the first test failure it encounters. Corresponding `after()` and `afterEach()` hooks are executed for potential cleanup. `--bail` does *not* imply `--exit`. diff --git a/lib/runner.js b/lib/runner.js index 315c953319..604d79b495 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -241,16 +241,14 @@ Runner.prototype.fail = function(test, err) { } this.emit('fail', test, err); - if (this.suite.bail()) { - this.emit('end'); - } }; /** * Fail the given `hook` with `err`. * * Hook failures work in the following pattern: - * - If bail, then exit + * - If bail, run corresponding `after each` and `after` hooks, + * then exit * - Failed `before` hook skips all tests in a suite and subsuites, * but jumps to corresponding `after` hook * - Failed `before each` hook skips remaining tests in a @@ -494,7 +492,7 @@ Runner.prototype.runTests = function(suite, fn) { function next(err, errSuite) { // if we bail after first err if (self.failures && suite._bail) { - return fn(); + tests = []; } if (self._abort) { diff --git a/test/integration/fixtures/options/bail-async.fixture.js b/test/integration/fixtures/options/bail-async.fixture.js new file mode 100644 index 0000000000..5095e63158 --- /dev/null +++ b/test/integration/fixtures/options/bail-async.fixture.js @@ -0,0 +1,23 @@ +'use strict'; + +describe('suite1', function () { + it('should display this spec', function () {}); + + it('should only display this error', function (done) { + throw new Error('this should be displayed'); + }); + + it('should not display this error', function (done) { + throw new Error('this should not be displayed'); + }); +}); + +describe('suite2', function () { + before(function (done) { + throw new Error('this hook should not be displayed'); + }); + + it('should not display this error', function (done) { + throw new Error('this should not be displayed'); + }); +}); diff --git a/test/integration/fixtures/options/bail-with-after.fixture.js b/test/integration/fixtures/options/bail-with-after.fixture.js index f722092924..2c1438eef1 100644 --- a/test/integration/fixtures/options/bail-with-after.fixture.js +++ b/test/integration/fixtures/options/bail-with-after.fixture.js @@ -1,11 +1,57 @@ 'use strict'; +var assert = require('assert'); describe('suite1', function () { - it('should only display this error', function () { - throw new Error('this should be displayed'); + var runOrder = []; + before('before suite1', function () { + runOrder.push('before suite1'); }); + beforeEach('beforeEach suite1', function () { + runOrder.push('beforeEach suite1'); + }); + it('test suite1', function () { + runOrder.push('test suite1'); + }); + + describe('suite1A', function () { + before('before suite1A', function () { + runOrder.push('before suite1A'); + }); + beforeEach('beforeEach suite1A', function () { + runOrder.push('beforeEach suite1A'); + }); + it('test suite1A', function () { + runOrder.push('test suite1A'); + }); + afterEach('afterEach suite1A', function () { + runOrder.push('afterEach suite1A'); + }); + after('after suite1A', function () { + runOrder.push('after suite1A'); + throw new Error('after suite1A error'); + }); + }); + + afterEach('afterEach suite1', function () { + runOrder.push('afterEach suite1'); + }); + after('after suite1', function () { + runOrder.push('after suite1'); + assert.deepStrictEqual(runOrder, [ + 'before suite1', 'beforeEach suite1', 'test suite1', + 'afterEach suite1', 'before suite1A', 'beforeEach suite1', + 'beforeEach suite1A', 'test suite1A', 'afterEach suite1A', + 'afterEach suite1', 'after suite1A', 'after suite1' + ]); + }); +}); - after(function () { - throw new Error('this hook should not be displayed'); +describe('suite2', function () { + before('before suite2', function () {}); + beforeEach('beforeEach suite2', function () {}); + it('test suite2', function () { + runOrder.push('test suite2 - should not run'); }); + afterEach('afterEach suite2', function () {}); + after('after suite2', function () {}); }); diff --git a/test/integration/fixtures/options/bail-with-afterEach.fixture.js b/test/integration/fixtures/options/bail-with-afterEach.fixture.js new file mode 100644 index 0000000000..4589bf1db8 --- /dev/null +++ b/test/integration/fixtures/options/bail-with-afterEach.fixture.js @@ -0,0 +1,57 @@ +'use strict'; +var assert = require('assert'); + +describe('suite1', function () { + var runOrder = []; + before('before suite1', function () { + runOrder.push('before suite1'); + }); + beforeEach('beforeEach suite1', function () { + runOrder.push('beforeEach suite1'); + }); + it('test suite1', function () { + runOrder.push('test suite1'); + }); + + describe('suite1A', function () { + before('before suite1A', function () { + runOrder.push('before suite1A'); + }); + beforeEach('beforeEach suite1A', function () { + runOrder.push('beforeEach suite1A'); + }); + it('test suite1A', function () { + runOrder.push('test suite1A'); + }); + afterEach('afterEach suite1A', function () { + runOrder.push('afterEach suite1A'); + throw new Error('afterEach suite1A error'); + }); + after('after suite1A', function () { + runOrder.push('after suite1A'); + }); + }); + + afterEach('afterEach suite1', function () { + runOrder.push('afterEach suite1'); + }); + after('after suite1', function () { + runOrder.push('after suite1'); + assert.deepStrictEqual(runOrder, [ + 'before suite1', 'beforeEach suite1', 'test suite1', + 'afterEach suite1', 'before suite1A', 'beforeEach suite1', + 'beforeEach suite1A', 'test suite1A', 'afterEach suite1A', + 'afterEach suite1', 'after suite1A', 'after suite1' + ]); + }); +}); + +describe('suite2', function () { + before('before suite2', function () {}); + beforeEach('beforeEach suite2', function () {}); + it('test suite2', function () { + runOrder.push('test suite2 - should not run'); + }); + afterEach('afterEach suite2', function () {}); + after('after suite2', function () {}); +}); diff --git a/test/integration/fixtures/options/bail-with-before.fixture.js b/test/integration/fixtures/options/bail-with-before.fixture.js index dfc0f8d939..3e885db0e3 100644 --- a/test/integration/fixtures/options/bail-with-before.fixture.js +++ b/test/integration/fixtures/options/bail-with-before.fixture.js @@ -1,11 +1,44 @@ 'use strict'; +var assert = require('assert'); describe('suite1', function () { - before(function () { - throw new Error('this hook should be only displayed'); + var runOrder = []; + before('before suite1', function () { + runOrder.push('before suite1'); + throw new Error('before suite1 error'); }); + beforeEach('beforeEach suite1', function () { + runOrder.push('beforeEach suite1 - should not run'); + }); + it('test suite1', function () { + runOrder.push('test suite1 - should not run'); + }); + + describe('suite1A', function () { + before('before suite1A', function () {}); + beforeEach('beforeEach suite1A', function () {}); + it('test suite1A', function () { + runOrder.push('test suite1A - should not run'); + }); + afterEach('afterEach suite1A', function () {}); + after('after suite1A', function () {}); + }); + + afterEach('afterEach suite1', function () { + runOrder.push('afterEach suite1 - should not run'); + }); + after('after suite1', function () { + runOrder.push('after suite1'); + assert.deepStrictEqual(runOrder, ['before suite1', 'after suite1']); + }); +}); - it('should not display this error', function () { - throw new Error('this should not be displayed'); +describe('suite2', function () { + before('before suite2', function () {}); + beforeEach('beforeEach suite2', function () {}); + it('test suite2', function () { + runOrder.push('test suite2 - should not run'); }); + afterEach('afterEach suite2', function () {}); + after('after suite2', function () {}); }); diff --git a/test/integration/fixtures/options/bail-with-beforeEach.fixture.js b/test/integration/fixtures/options/bail-with-beforeEach.fixture.js new file mode 100644 index 0000000000..0b390cbf43 --- /dev/null +++ b/test/integration/fixtures/options/bail-with-beforeEach.fixture.js @@ -0,0 +1,46 @@ +'use strict'; +var assert = require('assert'); + +describe('suite1', function () { + var runOrder = []; + before('before suite1', function () { + runOrder.push('before suite1'); + }); + beforeEach('beforeEach suite1', function () { + runOrder.push('beforeEach suite1'); + throw new Error('beforeEach suite1 error'); + }); + it('test suite1', function () { + runOrder.push('test suite1 - should not run'); + }); + + describe('suite1A', function () { + before('before suite1A', function () {}); + beforeEach('beforeEach suite1A', function () {}); + it('test suite1A', function () { + runOrder.push('test suite1A - should not run'); + }); + afterEach('afterEach suite1A', function () {}); + after('after suite1A', function () {}); + }); + + afterEach('afterEach suite1', function () { + runOrder.push('afterEach suite1'); + }); + after('after suite1', function () { + runOrder.push('after suite1'); + assert.deepStrictEqual(runOrder, [ + 'before suite1', 'beforeEach suite1', 'afterEach suite1', 'after suite1' + ]); + }); +}); + +describe('suite2', function () { + before('before suite2', function () {}); + beforeEach('beforeEach suite2', function () {}); + it('test suite2', function () { + runOrder.push('test suite2 - should not run'); + }); + afterEach('afterEach suite2', function () {}); + after('after suite2', function () {}); +}); diff --git a/test/integration/fixtures/options/bail-with-test.fixture.js b/test/integration/fixtures/options/bail-with-test.fixture.js new file mode 100644 index 0000000000..19d0181906 --- /dev/null +++ b/test/integration/fixtures/options/bail-with-test.fixture.js @@ -0,0 +1,47 @@ +'use strict'; +var assert = require('assert'); + +describe('suite1', function () { + var runOrder = []; + before('before suite1', function () { + runOrder.push('before suite1'); + }); + beforeEach('beforeEach suite1', function () { + runOrder.push('beforeEach suite1'); + }); + it('test suite1', function () { + runOrder.push('test suite1'); + throw new Error('test suite1 error'); + }); + + describe('suite1A', function () { + before('before suite1A', function () {}); + beforeEach('beforeEach suite1A', function () {}); + it('test suite1A', function () { + runOrder.push('test suite1A - should not run'); + }); + afterEach('afterEach suite1A', function () {}); + after('after suite1A', function () {}); + }); + + afterEach('afterEach suite1', function () { + runOrder.push('afterEach suite1'); + }); + after('after suite1', function () { + runOrder.push('after suite1'); + assert.deepStrictEqual(runOrder, [ + 'before suite1', 'beforeEach suite1', 'test suite1', + 'afterEach suite1', 'after suite1' + ]); + }); +}); + +describe('suite2', function () { + before('before suite2', function () {}); + beforeEach('beforeEach suite2', function () {}); + it('test suite2', function () { + runOrder.push('test suite2 - should not run'); + }); + afterEach('afterEach suite2', function () {}); + after('after suite2', function () {}); +}); diff --git a/test/integration/fixtures/options/bail.fixture.js b/test/integration/fixtures/options/bail.fixture.js index 5095e63158..4a5c091c25 100644 --- a/test/integration/fixtures/options/bail.fixture.js +++ b/test/integration/fixtures/options/bail.fixture.js @@ -3,21 +3,21 @@ describe('suite1', function () { it('should display this spec', function () {}); - it('should only display this error', function (done) { + it('should only display this error', function () { throw new Error('this should be displayed'); }); - it('should not display this error', function (done) { + it('should not display this error', function () { throw new Error('this should not be displayed'); }); }); describe('suite2', function () { - before(function (done) { + before(function () { throw new Error('this hook should not be displayed'); }); - it('should not display this error', function (done) { + it('should not display this error', function () { throw new Error('this should not be displayed'); }); }); diff --git a/test/integration/options.spec.js b/test/integration/options.spec.js index 0b94e85dde..3a161393d7 100644 --- a/test/integration/options.spec.js +++ b/test/integration/options.spec.js @@ -60,12 +60,29 @@ describe('options', function() { expect(res, 'to have failed') .and('to have passed test', 'should display this spec') .and('to have failed test', 'should only display this error') - .and('to have passed test count', 1); + .and('to have passed test count', 1) + .and('to have failed test count', 1); done(); }); }); - it('should stop all tests after the first error in before hook', function(done) { + it('should stop after the first error - async', function(done) { + runMochaJSON('options/bail-async.fixture.js', args, function(err, res) { + if (err) { + done(err); + return; + } + + expect(res, 'to have failed') + .and('to have passed test', 'should display this spec') + .and('to have failed test', 'should only display this error') + .and('to have passed test count', 1) + .and('to have failed test count', 1); + done(); + }); + }); + + it('should stop all tests after failing "before" hook', function(done) { runMochaJSON('options/bail-with-before.fixture.js', args, function( err, res @@ -76,12 +93,50 @@ describe('options', function() { } expect(res, 'to have failed') .and('to have failed test count', 1) - .and('to have failed test', '"before all" hook'); + .and('to have failed test', '"before all" hook: before suite1') + .and('to have passed test count', 0); + done(); + }); + }); + + it('should stop all tests after failing "beforeEach" hook', function(done) { + runMochaJSON('options/bail-with-beforeEach.fixture.js', args, function( + err, + res + ) { + if (err) { + done(err); + return; + } + expect(res, 'to have failed') + .and('to have failed test count', 1) + .and( + 'to have failed test', + '"before each" hook: beforeEach suite1 for "test suite1"' + ) + .and('to have passed test count', 0); done(); }); }); - it('should stop all hooks after the first error', function(done) { + it('should stop all tests after failing test', function(done) { + runMochaJSON('options/bail-with-test.fixture.js', args, function( + err, + res + ) { + if (err) { + done(err); + return; + } + expect(res, 'to have failed') + .and('to have failed test count', 1) + .and('to have failed test', 'test suite1') + .and('to have passed test count', 0); + done(); + }); + }); + + it('should stop all tests after failing "after" hook', function(done) { runMochaJSON('options/bail-with-after.fixture.js', args, function( err, res @@ -92,7 +147,30 @@ describe('options', function() { } expect(res, 'to have failed') .and('to have failed test count', 1) - .and('to have run test', 'should only display this error'); + .and('to have failed test', '"after all" hook: after suite1A') + .and('to have passed test count', 2) + .and('to have passed test order', 'test suite1', 'test suite1A'); + done(); + }); + }); + + it('should stop all tests after failing "afterEach" hook', function(done) { + runMochaJSON('options/bail-with-afterEach.fixture.js', args, function( + err, + res + ) { + if (err) { + done(err); + return; + } + expect(res, 'to have failed') + .and('to have failed test count', 1) + .and( + 'to have failed test', + '"after each" hook: afterEach suite1A for "test suite1A"' + ) + .and('to have passed test count', 2) + .and('to have passed test order', 'test suite1', 'test suite1A'); done(); }); }); diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 50af94aa5d..e89ef2b2d6 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -373,14 +373,6 @@ describe('Runner', function() { runner.failHook(hook, err); }); - it('should emit "end" if suite bail is true', function(done) { - var hook = new Hook(); - var err = {}; - suite.bail(true); - runner.on('end', done); - runner.failHook(hook, err); - }); - it('should not emit "end" if suite bail is not true', function(done) { var hook = new Hook(); var err = {};