From ebc6aee50dd1fdca9ab940b43ab79db72f0a4f93 Mon Sep 17 00:00:00 2001 From: Ajay Kodali Date: Wed, 15 Jul 2015 16:09:16 -0700 Subject: [PATCH] * Fixed async hook error handling issue where an error in the hook was bailing sibling suites too * Added unit test cases for async hook error handling * Fixed minor issue in existing sync hook error test case * Moving code to fixtures dir to make the hook.err.js tests more readable * Trimming the hook error test cases by removing hooks not under test * Fix indentation * Rename var to afterAllHookCalled --- lib/runner.js | 36 +- .../fixtures/hooks/after.hook.async.error.js | 19 + .../fixtures/hooks/after.hook.error.js | 17 + .../hooks/afterEach.hook.async.error.js | 19 + .../fixtures/hooks/afterEach.hook.error.js | 17 + .../fixtures/hooks/before.hook.async.error.js | 19 + .../fixtures/hooks/before.hook.error.js | 17 + .../hooks/beforeEach.hook.async.error.js | 19 + .../fixtures/hooks/beforeEach.hook.error.js | 17 + .../hooks/multiple.hook.async.error.js | 139 +++++++ .../fixtures/hooks/multiple.hook.error.js | 129 +++++++ test/integration/hook.err.js | 339 +++++------------- 12 files changed, 529 insertions(+), 258 deletions(-) create mode 100644 test/integration/fixtures/hooks/after.hook.async.error.js create mode 100644 test/integration/fixtures/hooks/after.hook.error.js create mode 100644 test/integration/fixtures/hooks/afterEach.hook.async.error.js create mode 100644 test/integration/fixtures/hooks/afterEach.hook.error.js create mode 100644 test/integration/fixtures/hooks/before.hook.async.error.js create mode 100644 test/integration/fixtures/hooks/before.hook.error.js create mode 100644 test/integration/fixtures/hooks/beforeEach.hook.async.error.js create mode 100644 test/integration/fixtures/hooks/beforeEach.hook.error.js create mode 100644 test/integration/fixtures/hooks/multiple.hook.async.error.js create mode 100644 test/integration/fixtures/hooks/multiple.hook.error.js diff --git a/lib/runner.js b/lib/runner.js index 1b5b4084cb..a63bb9ee82 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -538,6 +538,7 @@ Runner.prototype.runTests = function(suite, fn) { } this.next = next; + this.hookErr = hookErr; next(); }; @@ -552,6 +553,7 @@ Runner.prototype.runSuite = function(suite, fn) { var i = 0; var self = this; var total = this.grepTotal(suite); + var afterAllHookCalled = false; debug('run suite %s', suite.fullTitle()); @@ -597,12 +599,23 @@ Runner.prototype.runSuite = function(suite, fn) { function done(errSuite) { self.suite = suite; - self.hook('afterAll', function() { - self.emit('suite end', suite); + self.nextSuite = next; + + if (afterAllHookCalled) { fn(errSuite); - }); + } else { + // mark that the afterAll block has been called once + // and so can be skipped if there is an error in it. + afterAllHookCalled = true; + self.hook('afterAll', function() { + self.emit('suite end', suite); + fn(errSuite); + }); + } } + this.nextSuite = next; + this.hook('beforeAll', function(err) { if (err) { return done(); @@ -648,7 +661,22 @@ Runner.prototype.uncaught = function(err) { return; } - // bail on hooks + // recover from hooks + if (runnable.type === 'hook') { + var errSuite = this.suite; + // if hook failure is in afterEach block + if (runnable.fullTitle().indexOf('after each') > -1) { + return this.hookErr(err, errSuite, true); + } + // if hook failure is in beforeEach block + if (runnable.fullTitle().indexOf('before each') > -1) { + return this.hookErr(err, errSuite, false); + } + // if hook failure is in after or before blocks + return this.nextSuite(errSuite); + } + + // bail this.emit('end'); }; diff --git a/test/integration/fixtures/hooks/after.hook.async.error.js b/test/integration/fixtures/hooks/after.hook.async.error.js new file mode 100644 index 0000000000..dc55537a60 --- /dev/null +++ b/test/integration/fixtures/hooks/after.hook.async.error.js @@ -0,0 +1,19 @@ +describe('spec 1', function () { + after(function (done) { + console.log('after'); + process.nextTick(function () { + throw new Error('after hook error'); + }); + }); + it('should be called because error is in after hook', function () { + console.log('test 1'); + }); + it('should be called because error is in after hook', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/after.hook.error.js b/test/integration/fixtures/hooks/after.hook.error.js new file mode 100644 index 0000000000..e84856e433 --- /dev/null +++ b/test/integration/fixtures/hooks/after.hook.error.js @@ -0,0 +1,17 @@ +describe('spec 1', function () { + after(function () { + console.log('after'); + throw new Error('after hook error'); + }); + it('should be called because error is in after hook', function () { + console.log('test 1'); + }); + it('should be called because error is in after hook', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/afterEach.hook.async.error.js b/test/integration/fixtures/hooks/afterEach.hook.async.error.js new file mode 100644 index 0000000000..d4f4c9bdd9 --- /dev/null +++ b/test/integration/fixtures/hooks/afterEach.hook.async.error.js @@ -0,0 +1,19 @@ +describe('spec 1', function () { + afterEach(function (done) { + console.log('after'); + process.nextTick(function () { + throw new Error('after each hook error'); + }); + }); + it('should be called because error is in after each hook', function () { + console.log('test 1'); + }); + it('should not be called', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/afterEach.hook.error.js b/test/integration/fixtures/hooks/afterEach.hook.error.js new file mode 100644 index 0000000000..9d28632fa3 --- /dev/null +++ b/test/integration/fixtures/hooks/afterEach.hook.error.js @@ -0,0 +1,17 @@ +describe('spec 1', function () { + afterEach(function () { + console.log('after'); + throw new Error('after each hook error'); + }); + it('should be called because error is in after each hook', function () { + console.log('test 1'); + }); + it('should not be called', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/before.hook.async.error.js b/test/integration/fixtures/hooks/before.hook.async.error.js new file mode 100644 index 0000000000..10ea95156b --- /dev/null +++ b/test/integration/fixtures/hooks/before.hook.async.error.js @@ -0,0 +1,19 @@ +describe('spec 1', function () { + before(function (done) { + console.log('before'); + process.nextTick(function () { + throw new Error('before hook error'); + }); + }); + it('should not be called because of error in before hook', function () { + console.log('test 1'); + }); + it('should not be called because of error in before hook', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/before.hook.error.js b/test/integration/fixtures/hooks/before.hook.error.js new file mode 100644 index 0000000000..b3d6ea6640 --- /dev/null +++ b/test/integration/fixtures/hooks/before.hook.error.js @@ -0,0 +1,17 @@ +describe('spec 1', function () { + before(function () { + console.log('before'); + throw new Error('before hook error'); + }); + it('should not be called because of error in before hook', function () { + console.log('test 1'); + }); + it('should not be called because of error in before hook', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/beforeEach.hook.async.error.js b/test/integration/fixtures/hooks/beforeEach.hook.async.error.js new file mode 100644 index 0000000000..a34ed755e2 --- /dev/null +++ b/test/integration/fixtures/hooks/beforeEach.hook.async.error.js @@ -0,0 +1,19 @@ +describe('spec 1', function () { + beforeEach(function (done) { + console.log('before'); + process.nextTick(function () { + throw new Error('before each hook error'); + }); + }); + it('should not be called because of error in before each hook', function () { + console.log('test 1'); + }); + it('should not be called because of error in before each hook', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/beforeEach.hook.error.js b/test/integration/fixtures/hooks/beforeEach.hook.error.js new file mode 100644 index 0000000000..bd84e8fe74 --- /dev/null +++ b/test/integration/fixtures/hooks/beforeEach.hook.error.js @@ -0,0 +1,17 @@ +describe('spec 1', function () { + beforeEach(function () { + console.log('before'); + throw new Error('before each hook error'); + }); + it('should not be called because of error in before each hook', function () { + console.log('test 1'); + }); + it('should not be called because of error in before each hook', function () { + console.log('test 2'); + }); +}); +describe('spec 2', function () { + it('should be called, because hook error was in a sibling suite', function () { + console.log('test 3'); + }); +}); diff --git a/test/integration/fixtures/hooks/multiple.hook.async.error.js b/test/integration/fixtures/hooks/multiple.hook.async.error.js new file mode 100644 index 0000000000..c5474ad99b --- /dev/null +++ b/test/integration/fixtures/hooks/multiple.hook.async.error.js @@ -0,0 +1,139 @@ +before(function () { + console.log('root before'); +}); +beforeEach(function () { + console.log('root before each'); +}); +describe('1', function () { + beforeEach(function () { + console.log('1 before each'); + }); + + describe('1.1', function () { + before(function () { + console.log('1.1 before'); + }); + beforeEach(function (done) { + console.log('1.1 before each'); + process.nextTick(function () { + throw new Error('1.1 before each hook failed'); + }); + }); + it('1.1 test 1', function () { + console.log('1.1 test 1'); + }); + it('1.1 test 2', function () { + console.log('1.1 test 2'); + }); + afterEach(function () { + console.log('1.1 after each'); + }); + after(function (done) { + console.log('1.1 after'); + process.nextTick(function () { + throw new Error('1.1 after hook failed'); + }); + }); + }); + + describe('1.2', function () { + before(function () { + console.log('1.2 before'); + }); + beforeEach(function () { + console.log('1.2 before each'); + }); + it('1.2 test 1', function () { + console.log('1.2 test 1'); + }); + it('1.2 test 2', function () { + console.log('1.2 test 2'); + }); + afterEach(function (done) { + console.log('1.2 after each'); + process.nextTick(function () { + throw new Error('1.2 after each hook failed'); + }); + }); + after(function () { + console.log('1.2 after'); + }); + }); + + afterEach(function () { + console.log('1 after each'); + }); + + after(function () { + console.log('1 after'); + }); +}); + +describe('2', function () { + beforeEach(function (done) { + console.log('2 before each'); + process.nextTick(function () { + throw new Error('2 before each hook failed'); + }); + }); + + describe('2.1', function () { + before(function () { + console.log('2.1 before'); + }); + beforeEach(function () { + console.log('2.1 before each'); + }); + it('2.1 test 1', function () { + console.log('2.1 test 1'); + }); + it('2.1 test 2', function () { + console.log('2.1 test 2'); + }); + afterEach(function () { + console.log('2.1 after each'); + }); + after(function () { + console.log('2.1 after'); + }); + }); + + describe('2.2', function () { + before(function () { + console.log('2.2 before'); + }); + beforeEach(function () { + console.log('2.2 before each'); + }); + it('2.2 test 1', function () { + console.log('2.2 test 1'); + }); + it('2.2 test 2', function () { + console.log('2.2 test 2'); + }); + afterEach(function () { + console.log('2.2 after each'); + }); + after(function () { + console.log('2.2 after'); + }); + }); + + afterEach(function (done) { + console.log('2 after each'); + process.nextTick(function () { + throw new Error('2 after each hook failed'); + }); + }); + + after(function () { + console.log('2 after'); + }); +}); + +after(function () { + console.log('root after'); +}); +afterEach(function () { + console.log('root after each'); +}); diff --git a/test/integration/fixtures/hooks/multiple.hook.error.js b/test/integration/fixtures/hooks/multiple.hook.error.js new file mode 100644 index 0000000000..f0983cf42f --- /dev/null +++ b/test/integration/fixtures/hooks/multiple.hook.error.js @@ -0,0 +1,129 @@ +before(function () { + console.log('root before'); +}); +beforeEach(function () { + console.log('root before each'); +}); +describe('1', function () { + beforeEach(function () { + console.log('1 before each'); + }); + + describe('1.1', function () { + before(function () { + console.log('1.1 before'); + }); + beforeEach(function () { + console.log('1.1 before each'); + throw new Error('1.1 before each hook failed'); + }); + it('1.1 test 1', function () { + console.log('1.1 test 1'); + }); + it('1.1 test 2', function () { + console.log('1.1 test 2'); + }); + afterEach(function () { + console.log('1.1 after each'); + }); + after(function () { + console.log('1.1 after'); + throw new Error('1.1 after hook failed'); + }); + }); + + describe('1.2', function () { + before(function () { + console.log('1.2 before'); + }); + beforeEach(function () { + console.log('1.2 before each'); + }); + it('1.2 test 1', function () { + console.log('1.2 test 1'); + }); + it('1.2 test 2', function () { + console.log('1.2 test 2'); + }); + afterEach(function () { + console.log('1.2 after each'); + throw new Error('1.2 after each hook failed'); + }); + after(function () { + console.log('1.2 after'); + }); + }); + + afterEach(function () { + console.log('1 after each'); + }); + + after(function () { + console.log('1 after'); + }); +}); + +describe('2', function () { + beforeEach(function () { + console.log('2 before each'); + throw new Error('2 before each hook failed'); + }); + + describe('2.1', function () { + before(function () { + console.log('2.1 before'); + }); + beforeEach(function () { + console.log('2.1 before each'); + }); + it('2.1 test 1', function () { + console.log('2.1 test 1'); + }); + it('2.1 test 2', function () { + console.log('2.1 test 2'); + }); + afterEach(function () { + console.log('2.1 after each'); + }); + after(function () { + console.log('2.1 after'); + }); + }); + + describe('2.2', function () { + before(function () { + console.log('2.2 before'); + }); + beforeEach(function () { + console.log('2.2 before each'); + }); + it('2.2 test 1', function () { + console.log('2.2 test 1'); + }); + it('2.2 test 2', function () { + console.log('2.2 test 2'); + }); + afterEach(function () { + console.log('2.2 after each'); + }); + after(function () { + console.log('2.2 after'); + }); + }); + + afterEach(function () { + console.log('2 after each'); + throw new Error('2 after each hook failed'); + }); + + after(function () { + console.log('2 after'); + }); +}); + +after(function () { + console.log('root after'); +}); +afterEach(function () { + console.log('root after each'); +}); diff --git a/test/integration/hook.err.js b/test/integration/hook.err.js index 0e3329523e..a3727156ff 100644 --- a/test/integration/hook.err.js +++ b/test/integration/hook.err.js @@ -1,5 +1,5 @@ var assert = require('assert'); -var runMochaFunction = require('./helpers').runMochaFunction; +var runMocha = require('./helpers').runMocha; describe('hook error handling', function() { this.timeout(1000); @@ -7,294 +7,125 @@ describe('hook error handling', function() { var lines; describe('before hook error', function() { - before(run(function beforeHookError() { - describe('spec 1', function() { - describe('spec 1 nested', function() { - it('should not be called, because hook error was in a parent suite', function() { - console.log('test nested'); - }); - }); - before(function() { - console.log('before'); - throw new Error('before hook error'); - }); - after(function() { - console.log('after'); - }); - it('should not be called because of error in before hook', function() { - console.log('test'); - }); - }); - describe('spec 2', function() { - before(function() { - console.log('before 2'); - }); - after(function() { - console.log('after 2'); - }); - it('should be called, because hook error was in a sibling suite', function() { - console.log('test 2'); - }); - }); - })); - + before(run('hooks/before.hook.error.js')); it('should verify results', function() { assert.deepEqual( lines, - ['before', 'after', 'before 2', 'test 2', 'after 2'] + ['before', 'test 3'] ); }); }); describe('before each hook error', function() { - before(run(function beforeEachHookError() { - describe('spec 1', function() { - describe('spec 1 nested', function() { - it('should not be called, because hook error was in a parent suite', function() { - console.log('test nested'); - }); - }); - beforeEach(function() { - console.log('before'); - throw new Error('before each hook error'); - }); - afterEach(function() { - console.log('after'); - }); - it('should not be called because of error in before each hook', function() { - console.log('test'); - }); - }); - describe('spec 2', function() { - before(function() { - console.log('before 2'); - }); - after(function() { - console.log('after 2'); - }); - it('should be called, because hook error was in a sibling suite', function() { - console.log('test 2'); - }); - }); - })); + before(run('hooks/beforeEach.hook.error.js')); it('should verify results', function() { assert.deepEqual( lines, - ['before', 'after', 'before 2', 'test 2', 'after 2'] + ['before', 'test 3'] ); }); }); describe('after hook error', function() { - before(run(function afterHookError() { - describe('spec 1', function() { - describe('spec 1 nested', function() { - it('should be called, because hook error will happen after parent suite', function() { - console.log('test nested'); - }); - }); - before(function() { - console.log('before'); - }); - after(function() { - console.log('after'); - throw new Error('after hook error'); - }); - it('should be called because error is in after hook', function() { - console.log('test'); - }); - }); - describe('spec 2', function() { - before(function() { - console.log('before 2'); - }); - after(function() { - console.log('after 2'); - }); - it('should be called, because hook error was in a sibling suite', function() { - console.log('test 2'); - }); - }); - })); + before(run('hooks/after.hook.error.js')); it('should verify results', function() { assert.deepEqual( lines, - ['before', 'test', 'test nested', 'after', 'before 2', 'test 2', 'after 2'] + ['test 1', 'test 2', 'after', 'test 3'] ); }); }); describe('after each hook error', function() { - before(run(function afterEachHookError() { - describe('spec 1', function() { - describe('spec 1 nested', function() { - it('should be called, because hook error will happen after parent suite', function() { - console.log('test nested'); - }); - }); - before(function() { - console.log('before'); - }); - after(function() { - console.log('after'); - throw new Error('after hook error'); - }); - it('should be called because error is in after hook', function() { - console.log('test'); - }); - }); - describe('spec 2', function() { - before(function() { - console.log('before 2'); - }); - after(function() { - console.log('after 2'); - }); - it('should be called, because hook error was in a sibling suite', function() { - console.log('test 2'); - }); - }); - })); + before(run('hooks/afterEach.hook.error.js')); it('should verify results', function() { assert.deepEqual( lines, - ['before', 'test', 'test nested', 'after', 'before 2', 'test 2', 'after 2'] + ['test 1', 'after', 'test 3'] ); }); }); describe('multiple hook errors', function() { - before(run(function multipleHookErrors() { - before(function() { - console.log('root before'); - }); - beforeEach(function() { - console.log('root before each'); - }); - describe('1', function() { - beforeEach(function() { - console.log('1 before each'); - }); - - describe('1.1', function() { - before(function() { - console.log('1.1 before'); - }); - beforeEach(function() { - console.log('1.1 before each'); - throw new Error('1.1 before each hook failed'); - }); - it('1.1 test 1', function() { - console.log('1.1 test 1'); - }); - it('1.1 test 2', function() { - console.log('1.1 test 2'); - }); - afterEach(function() { - console.log('1.1 after each'); - }); - after(function() { - console.log('1.1 after'); - throw new Error('1.1 after hook failed'); - }); - }); - - describe('1.2', function() { - before(function() { - console.log('1.2 before'); - }); - beforeEach(function() { - console.log('1.2 before each'); - }); - it('1.2 test 1', function() { - console.log('1.2 test 1'); - }); - it('1.2 test 2', function() { - console.log('1.2 test 2'); - }); - afterEach(function() { - console.log('1.2 after each'); - throw new Error('1.2 after each hook failed'); - }); - after(function() { - console.log('1.2 after'); - }); - }); - - afterEach(function() { - console.log('1 after each'); - }); - - after(function() { - console.log('1 after'); - }); - }); - - describe('2', function() { - beforeEach(function() { - console.log('2 before each'); - throw new Error('2 before each hook failed'); - }); + before(run('hooks/multiple.hook.error.js')); + it('should verify results', function() { + assert.deepEqual( + lines, + [ + 'root before', + '1.1 before', + 'root before each', + '1 before each', + '1.1 before each', + '1.1 after each', + '1 after each', + 'root after each', + '1.1 after', + '1.2 before', + 'root before each', + '1 before each', + '1.2 before each', + '1.2 test 1', + '1.2 after each', + '1 after each', + 'root after each', + '1.2 after', + '1 after', + '2.1 before', + 'root before each', + '2 before each', + '2 after each', + 'root after each', + '2.1 after', + '2 after', + 'root after' + ] + ); + }); + }); - describe('2.1', function() { - before(function() { - console.log('2.1 before'); - }); - beforeEach(function() { - console.log('2.1 before each'); - }); - it('2.1 test 1', function() { - console.log('2.1 test 1'); - }); - it('2.1 test 2', function() { - console.log('2.1 test 2'); - }); - afterEach(function() { - console.log('2.1 after each'); - }); - after(function() { - console.log('2.1 after'); - }); - }); + describe('async - before hook error', function() { + before(run('hooks/before.hook.async.error.js')); + it('should verify results', function() { + assert.deepEqual( + lines, + ['before', 'test 3'] + ); + }); + }); - describe('2.2', function() { - before(function() { - console.log('2.2 before'); - }); - beforeEach(function() { - console.log('2.2 before each'); - }); - it('2.2 test 1', function() { - console.log('2.2 test 1'); - }); - it('2.2 test 2', function() { - console.log('2.2 test 2'); - }); - afterEach(function() { - console.log('2.2 after each'); - }); - after(function() { - console.log('2.2 after'); - }); - }); + describe('async - before each hook error', function() { + before(run('hooks/beforeEach.hook.async.error.js')); + it('should verify results', function() { + assert.deepEqual( + lines, + ['before', 'test 3'] + ); + }); + }); - afterEach(function() { - console.log('2 after each'); - throw new Error('2 after each hook failed'); - }); + describe('async - after hook error', function() { + before(run('hooks/after.hook.async.error.js')); + it('should verify results', function() { + assert.deepEqual( + lines, + ['test 1', 'test 2', 'after', 'test 3'] + ); + }); + }); - after(function() { - console.log('2 after'); - }); - }); + describe('async - after each hook error', function() { + before(run('hooks/afterEach.hook.async.error.js')); + it('should verify results', function() { + assert.deepEqual( + lines, + ['test 1', 'after', 'test 3'] + ); + }); + }); - after(function() { - console.log('root after'); - }); - afterEach(function() { - console.log('root after each'); - }); - })); + describe('async - multiple hook errors', function() { + before(run('hooks/multiple.hook.async.error.js')); it('should verify results', function() { assert.deepEqual( lines, @@ -331,9 +162,9 @@ describe('hook error handling', function() { }); }); - function run(fn) { + function run(fnPath) { return function(done) { - runMochaFunction(fn, [], function(err, res) { + runMocha(fnPath, [], function(err, res) { assert.ifError(err); lines = res.output