diff --git a/lib/runner.js b/lib/runner.js index 6f7e28ca77..f01fc6aeee 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -201,10 +201,18 @@ Runner.prototype.fail = function(test, err){ /** * Fail the given `hook` with `err`. * - * Hook failures (currently) hard-end due - * to that fact that a failing hook will - * surely cause subsequent tests to fail, - * causing jumbled reporting. + * Hook failures work in the following pattern: + * - If bail, 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 + * suite and jumps to corresponding `after each` hook, + * which is run only once + * - Failed `after` hook does not alter + * execution order + * - Failed `after each` hook skips remaining tests in a + * suite and subsuites, but executes other `after each` + * hooks * * @param {Hook} hook * @param {Error} err @@ -213,7 +221,9 @@ Runner.prototype.fail = function(test, err){ Runner.prototype.failHook = function(hook, err){ this.fail(hook, err); - this.emit('end'); + if (this.suite.bail()) { + this.emit('end'); + } }; /** @@ -248,7 +258,12 @@ Runner.prototype.hook = function(name, fn){ hook.removeAllListeners('error'); var testError = hook.error(); if (testError) self.fail(self.test, testError); - if (err) return self.failHook(hook, err); + if (err) { + self.failHook(hook, err); + + // stop executing hooks, notify callee of hook err + return fn(err); + } self.emit('hook end', hook); delete hook.ctx.currentTest; next(++i); @@ -262,7 +277,7 @@ Runner.prototype.hook = function(name, fn){ /** * Run hook `name` for the given array of `suites` - * in order, and callback `fn(err)`. + * in order, and callback `fn(err, errSuite)`. * * @param {String} name * @param {Array} suites @@ -284,8 +299,9 @@ Runner.prototype.hooks = function(name, suites, fn){ self.hook(name, function(err){ if (err) { + var errSuite = self.suite; self.suite = orig; - return fn(err); + return fn(err, errSuite); } next(suites.pop()); @@ -373,10 +389,37 @@ Runner.prototype.runTests = function(suite, fn){ , tests = suite.tests.slice() , test; - function next(err) { + + function hookErr(err, errSuite, after) { + // before/after Each hook for errSuite failed: + var orig = self.suite; + + // for failed 'after each' hook start from errSuite parent, + // otherwise start from errSuite itself + self.suite = after ? errSuite.parent : errSuite; + + if (self.suite) { + // call hookUp afterEach + self.hookUp('afterEach', function(err2, errSuite2) { + self.suite = orig; + // some hooks may fail even now + if (err2) return hookErr(err2, errSuite2, true); + // report error suite + fn(errSuite); + }); + } else { + // there is no need calling other 'after each' hooks + self.suite = orig; + fn(errSuite); + } + } + + function next(err, errSuite) { // if we bail after first err if (self.failures && suite._bail) return fn(); + if (err) return hookErr(err, errSuite, true); + // next test test = tests.shift(); @@ -397,7 +440,10 @@ Runner.prototype.runTests = function(suite, fn){ // execute test and hook(s) self.emit('test', self.test = test); - self.hookDown('beforeEach', function(){ + self.hookDown('beforeEach', function(err, errSuite){ + + if (err) return hookErr(err, errSuite, false); + self.currentRunnable = self.test; self.runTest(function(err){ test = self.test; @@ -440,21 +486,35 @@ Runner.prototype.runSuite = function(suite, fn){ this.emit('suite', this.suite = suite); - function next() { + function next(errSuite) { + if (errSuite) { + // current suite failed on a hook from errSuite + if (errSuite == suite) { + // if errSuite is current suite + // continue to the next sibling suite + return done(); + } else { + // errSuite is among the parents of current suite + // stop execution of errSuite and all sub-suites + return done(errSuite); + } + } + var curr = suite.suites[i++]; if (!curr) return done(); self.runSuite(curr, next); } - function done() { + function done(errSuite) { self.suite = suite; self.hook('afterAll', function(){ self.emit('suite end', suite); - fn(); + fn(errSuite); }); } - this.hook('beforeAll', function(){ + this.hook('beforeAll', function(err){ + if (err) return done(); self.runTests(suite, next); }); }; diff --git a/mocha.js b/mocha.js index 9b7c1fa56d..16fbb8a305 100644 --- a/mocha.js +++ b/mocha.js @@ -4422,9 +4422,7 @@ Runner.prototype.globalProps = function() { Runner.prototype.globals = function(arr){ if (0 == arguments.length) return this._globals; debug('globals %j', arr); - utils.forEach(arr, function(arr){ - this._globals.push(arr); - }, this); + this._globals = this._globals.concat(arr); return this; }; @@ -4480,10 +4478,18 @@ Runner.prototype.fail = function(test, err){ /** * Fail the given `hook` with `err`. * - * Hook failures (currently) hard-end due - * to that fact that a failing hook will - * surely cause subsequent tests to fail, - * causing jumbled reporting. + * Hook failures work in the following pattern: + * - If bail, 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 + * suite and jumps to corresponding `after each` hook, + * which is run only once + * - Failed `after` hook does not alter + * execution order + * - Failed `after each` hook skips remaining tests in a + * suite and subsuites, but executes other `after each` + * hooks * * @param {Hook} hook * @param {Error} err @@ -4492,7 +4498,9 @@ Runner.prototype.fail = function(test, err){ Runner.prototype.failHook = function(hook, err){ this.fail(hook, err); - this.emit('end'); + if (this.suite.bail()) { + this.emit('end'); + } }; /** @@ -4527,7 +4535,12 @@ Runner.prototype.hook = function(name, fn){ hook.removeAllListeners('error'); var testError = hook.error(); if (testError) self.fail(self.test, testError); - if (err) return self.failHook(hook, err); + if (err) { + self.failHook(hook, err); + + // stop executing hooks, notify callee of hook err + return fn(err); + } self.emit('hook end', hook); delete hook.ctx.currentTest; next(++i); @@ -4541,7 +4554,7 @@ Runner.prototype.hook = function(name, fn){ /** * Run hook `name` for the given array of `suites` - * in order, and callback `fn(err)`. + * in order, and callback `fn(err, errSuite)`. * * @param {String} name * @param {Array} suites @@ -4563,8 +4576,9 @@ Runner.prototype.hooks = function(name, suites, fn){ self.hook(name, function(err){ if (err) { + var errSuite = self.suite; self.suite = orig; - return fn(err); + return fn(err, errSuite); } next(suites.pop()); @@ -4652,10 +4666,37 @@ Runner.prototype.runTests = function(suite, fn){ , tests = suite.tests.slice() , test; - function next(err) { + + function hookErr(err, errSuite, after) { + // before/after Each hook for errSuite failed: + var orig = self.suite; + + // for failed 'after each' hook start from errSuite parent, + // otherwise start from errSuite itself + self.suite = after ? errSuite.parent : errSuite; + + if (self.suite) { + // call hookUp afterEach + self.hookUp('afterEach', function(err2, errSuite2) { + self.suite = orig; + // some hooks may fail even now + if (err2) return hookErr(err2, errSuite2, true); + // report error suite + fn(errSuite); + }); + } else { + // there is no need calling other 'after each' hooks + self.suite = orig; + fn(errSuite); + } + } + + function next(err, errSuite) { // if we bail after first err if (self.failures && suite._bail) return fn(); + if (err) return hookErr(err, errSuite, true); + // next test test = tests.shift(); @@ -4676,7 +4717,10 @@ Runner.prototype.runTests = function(suite, fn){ // execute test and hook(s) self.emit('test', self.test = test); - self.hookDown('beforeEach', function(){ + self.hookDown('beforeEach', function(err, errSuite){ + + if (err) return hookErr(err, errSuite, false); + self.currentRunnable = self.test; self.runTest(function(err){ test = self.test; @@ -4719,21 +4763,35 @@ Runner.prototype.runSuite = function(suite, fn){ this.emit('suite', this.suite = suite); - function next() { + function next(errSuite) { + if (errSuite) { + // current suite failed on a hook from errSuite + if (errSuite == suite) { + // if errSuite is current suite + // continue to the next sibling suite + return done(); + } else { + // errSuite is among the parents of current suite + // stop execution of errSuite and all sub-suites + return done(errSuite); + } + } + var curr = suite.suites[i++]; if (!curr) return done(); self.runSuite(curr, next); } - function done() { + function done(errSuite) { self.suite = suite; self.hook('afterAll', function(){ self.emit('suite end', suite); - fn(); + fn(errSuite); }); } - this.hook('beforeAll', function(){ + this.hook('beforeAll', function(err){ + if (err) return done(); self.runTests(suite, next); }); }; diff --git a/test/hook.err.js b/test/hook.err.js new file mode 100644 index 0000000000..f28ecdd501 --- /dev/null +++ b/test/hook.err.js @@ -0,0 +1,296 @@ +describe('hook error handling', function(){ + // Lines in this test should be uncommented to see actual behavior + // You will also see errors in hooks + describe('before hook error', function() { + var calls = []; + describe('spec 1', function () { + describe('spec 1 nested', function () { + it('should not be called, because hook error was in a parent suite', function() { + calls.push('test nested'); + }) + }) + before(function(){ + calls.push('before'); + // throw new Error('before hook error'); + }) + after(function(){ + calls.push('after'); + }) + it('should not be called because of error in before hook', function() { + calls.push('test'); + }) + }) + describe('spec 2', function () { + before(function(){ + calls.push('before 2'); + }) + after(function(){ + calls.push('after 2'); + }) + it('should be called, because hook error was in a sibling suite', function() { + calls.push('test 2'); + }) + }) + after(function () { + // calls.should.eql(['before', 'after', 'before 2', 'test 2', 'after 2']); + }) + }) + + describe('before each hook error', function() { + var calls = []; + describe('spec 1', function () { + describe('spec 1 nested', function () { + it('should not be called, because hook error was in a parent suite', function() { + calls.push('test nested'); + }) + }) + beforeEach(function(){ + calls.push('before'); + // throw new Error('before each hook error'); + }) + afterEach(function(){ + calls.push('after'); + }) + it('should not be called because of error in before each hook', function() { + calls.push('test'); + }) + }) + describe('spec 2', function () { + before(function(){ + calls.push('before 2'); + }) + after(function(){ + calls.push('after 2'); + }) + it('should be called, because hook error was in a sibling suite', function() { + calls.push('test 2'); + }) + }) + after(function () { + // This should be called ! + // calls.should.eql(['before', 'after', 'before 2', 'test 2', 'after 2']); + }) + }) + + describe('after hook error', function() { + var calls = []; + describe('spec 1', function () { + describe('spec 1 nested', function () { + it('should be called, because hook error will happen after parent suite', function() { + calls.push('test nested'); + }) + }) + before(function(){ + calls.push('before'); + }) + after(function(){ + calls.push('after'); + // throw new Error('after hook error'); + }) + it('should be called because error is in after hook', function() { + calls.push('test'); + }) + }) + describe('spec 2', function () { + before(function(){ + calls.push('before 2'); + }) + after(function(){ + calls.push('after 2'); + }) + it('should be called, because hook error was in a sibling suite', function() { + calls.push('test 2'); + }) + }) + after(function () { + // Even this should be called ! + // calls.should.eql(['before', 'test', 'test nested', 'after', 'before 2', 'test 2', 'after 2']); + }) + }) + + describe('after each hook error', function() { + var calls = []; + describe('spec 1', function () { + describe('spec 1 nested', function () { + it('should not be called, because hook error has already happened in parent suite', function() { + calls.push('test nested'); + }) + }) + beforeEach(function(){ + calls.push('before'); + }) + afterEach(function(){ + calls.push('after'); + // throw new Error('after each hook error'); + }) + it('should be called because error is in after each hook, and this is the first test', function() { + calls.push('test'); + }) + it('should not be called because error is in after each hook, and this is the second test', function() { + calls.push('another test'); + }) + }) + describe('spec 2', function () { + before(function(){ + calls.push('before 2'); + }) + after(function(){ + calls.push('after 2'); + }) + it('should be called, because hook error was in a sibling suite', function() { + calls.push('test 2'); + }) + }) + after(function () { + // This should be called ! + // calls.should.eql(['before', 'test', 'after', 'before 2', 'test 2', 'after 2']); + }) + }) + + describe('multiple hook errors', function() { + var calls = []; + before(function(){ + calls.push("root before"); + }); + beforeEach(function(){ + calls.push("root before each"); + }); + describe('1', function(){ + beforeEach(function() { + calls.push('1 before each') + }) + + describe('1.1', function(){ + before(function() { + calls.push('1.1 before'); + }); + beforeEach(function() { + calls.push('1.1 before each') + // throw new Error('1.1 before each hook failed') + }); + it('1.1 test 1', function () {calls.push('1.1 test 1')}); + it('1.1 test 2', function () {calls.push('1.1 test 2')}); + afterEach(function() { + calls.push("1.1 after each"); + }); + after(function(){ + calls.push("1.1 after"); + // throw new Error('1.1 after hook failed') + }); + }); + + describe('1.2', function(){ + before(function() { + calls.push('1.2 before'); + }); + beforeEach(function() { + calls.push('1.2 before each') + }); + it('1.2 test 1', function () {calls.push('1.2 test 1')}); + it('1.2 test 2', function () {calls.push('1.2 test 2')}); + afterEach(function() { + calls.push("1.2 after each"); + // throw new Error('1.2 after each hook failed') + }); + after(function(){ + calls.push("1.2 after"); + }); + }); + + afterEach(function() { + calls.push('1 after each') + }) + + after(function(){ + calls.push("1 after"); + }); + }) + + describe('2', function(){ + beforeEach(function() { + calls.push('2 before each') + // throw new Error('2 before each hook failed') + }) + + describe('2.1', function(){ + before(function() { + calls.push('2.1 before'); + }); + beforeEach(function() { + calls.push('2.1 before each') + }); + it('2.1 test 1', function () {calls.push('2.1 test 1')}); + it('2.1 test 2', function () {calls.push('2.1 test 2')}); + afterEach(function() { + calls.push("2.1 after each"); + }); + after(function(){ + calls.push("2.1 after"); + }); + }); + + describe('2.2', function(){ + before(function() { + calls.push('2.2 before'); + }); + beforeEach(function() { + calls.push('2.2 before each') + }); + it('2.2 test 1', function () {calls.push('2.2 test 1')}); + it('2.2 test 2', function () {calls.push('2.2 test 2')}); + afterEach(function() { + calls.push("2.2 after each"); + }); + after(function(){ + calls.push("2.2 after"); + }); + }); + + afterEach(function() { + calls.push('2 after each') + // throw new Error('2 after each hook failed') + }) + + after(function(){ + calls.push("2 after"); + }); + }) + + after(function(){ + calls.push("root after"); + /* calls.should.eql([ + "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" + ]); */ + }); + afterEach(function(){ + calls.push("root after each"); + }); + }) + +}) \ No newline at end of file diff --git a/test/runner.js b/test/runner.js index 54d574c9de..98137399c8 100644 --- a/test/runner.js +++ b/test/runner.js @@ -160,7 +160,7 @@ describe('Runner', function(){ }) }) - describe('.failHook(hoot, err)', function(){ + describe('.failHook(hook, err)', function(){ it('should increment .failures', function(){ runner.failures.should.equal(0); runner.failHook({}, {}); @@ -179,10 +179,19 @@ describe('Runner', function(){ runner.failHook(hook, err); }) - it('should emit "end"', function(done){ + it('should emit "end" if suite bail is true', function(done){ var hook = {}, 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 = {}, err = {}; + suite.bail(false); + runner.on('end', function() { throw new Error('"end" was emit, but the bail is false'); }); + runner.failHook(hook, err); + done(); + }) }) })