Skip to content

Commit

Permalink
(re-)enable after each hooks to fail tests.
Browse files Browse the repository at this point in the history
In version 1.2.1, support was added to fail test in the after each hook:
> * Added `this.test.error(err)` support to after each hooks. Closes mochajs#287

While migrating from teaspoon-mocha to karma-mocha, we discovered that
in the current mocha version this isn't supported anymore (or we don't
understand how). In teaspoon this behaviour seems to be patched (running
it with the exact same mocha.js 2.2.4 file, teaspoon and karma have
different behaviour), although I couldn't quickly pinpoint the code that
does that.

This patch adds this behaviour back to the code base, but also seems to
break certain behaviour (3 tests are failing with this pull request).
But I don't understand what those test failures exactly mean. Also I
found it quite difficult to add test coverage of this behaviour. Since
there aren't any tests calling `runTests`.

Help in figuring out what we've done wrong or how we could fix this pull
request, would be appreciated. Not supporting this behaviour anymore due
to added complexity in the code, would also be fine, but adding some
documentation about dropping this functionality would be nice.
  • Loading branch information
jjoos committed Mar 17, 2016
1 parent 9ae6a85 commit 6f46073
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 7 deletions.
13 changes: 10 additions & 3 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -553,9 +553,16 @@ Runner.prototype.runTests = function(suite, fn) {
}

test.state = 'passed';
self.emit('pass', test);
self.emit('test end', test);
self.hookUp('afterEach', next);
// Run the actual emit that the test passed after the forEach hooks,
// so they can still fail a test. And it's not already reported as
// passed.
self.hookUp('afterEach', function() {
if (test.state === 'passed') {
self.emit('pass', test);
}
self.emit('test end', test);
next();
});
});
});
}
Expand Down
9 changes: 6 additions & 3 deletions test/integration/hook.err.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('hook error handling', function() {
it('should verify results', function() {
assert.deepEqual(
lines,
['test 1', 'after', 'test 3']
['test 1', 'after', 'test 2', 'after', 'test 3']
);
});
});
Expand All @@ -77,8 +77,11 @@ describe('hook error handling', function() {
'1.2 before each',
'1.2 test 1',
'1.2 after each',
'1 after each',
'root after each',
'root before each',
'1 before each',
'1.2 before each',
'1.2 test 2',
'1.2 after each',
'1.2 after',
'1 after',
'2.1 before',
Expand Down
2 changes: 1 addition & 1 deletion test/integration/multiple.done.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('multiple calls to done()', function() {

it('results in failures', function() {
assert.equal(res.stats.pending, 0);
assert.equal(res.stats.passes, 1);
assert.equal(res.stats.passes, 0);
assert.equal(res.stats.failures, 1);
assert.equal(res.code, 1);
});
Expand Down

0 comments on commit 6f46073

Please sign in to comment.