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.

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

Hook error handling (#534) -- continue testing even if some hooks are failing #1043

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 74 additions & 14 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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');
}
};

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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();

Expand All @@ -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;
Expand Down Expand Up @@ -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);
});
};
Expand Down
92 changes: 75 additions & 17 deletions mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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
Expand All @@ -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');
}
};

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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();

Expand All @@ -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;
Expand Down Expand Up @@ -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);
});
};
Expand Down
Loading