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.

Sign up for GitHub

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

Jump to bottom

Behavior of after/afterEach hooks with --bail flag #3617

Merged
merged 5 commits into from Dec 19, 2018
Merged

Behavior of after/afterEach hooks with --bail flag #3617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0545548
runner.js: delete second end emit
juergba Dec 14, 2018
ad69c37
tests
juergba Dec 18, 2018
4e43041
documentation
juergba Dec 18, 2018
eb3612e
runner.js
juergba Dec 18, 2018
8336eb9
tests: corrections
juergba Dec 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/index.md
View file
Open in desktop
Expand Up @@ -907,7 +907,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`.

Expand Down
8 changes: 3 additions & 5 deletions lib/runner.js
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
23 changes: 23 additions & 0 deletions test/integration/fixtures/options/bail-async.fixture.js
View file
Open in desktop
@@ -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');
});
});
54 changes: 50 additions & 4 deletions test/integration/fixtures/options/bail-with-after.fixture.js
View file
Open in desktop
@@ -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 () {});
});
57 changes: 57 additions & 0 deletions test/integration/fixtures/options/bail-with-afterEach.fixture.js
View file
Open in desktop
@@ -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 () {});
});
41 changes: 37 additions & 4 deletions test/integration/fixtures/options/bail-with-before.fixture.js
View file
Open in desktop
@@ -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 () {});
});
46 changes: 46 additions & 0 deletions test/integration/fixtures/options/bail-with-beforeEach.fixture.js
View file
Open in desktop
@@ -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 () {});
});
47 changes: 47 additions & 0 deletions test/integration/fixtures/options/bail-with-test.fixture.js
View file
Open in desktop
@@ -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 () {});
});
8 changes: 4 additions & 4 deletions test/integration/fixtures/options/bail.fixture.js
View file
Open in desktop
Expand Up @@ -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 () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this causing a problem? perhaps your changes should actually be additions; two tests that use done and two that don't. this would give us confidence that the same thing works properly regardless of the Runnable's strategy (async or sync)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, no problem arose, the 'suite2' describe is skipped (correctly) anyway. I added a second test, one sync and one async.

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');
});
});