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

[Rulikkk]

This closes #534 and also affects #973, #975 and maybe #516, #581, #664, etc.

I have implemented hook-error behaviour in a try-finally manner, which means that for the failing hook, its counterpart will be executed (if exists), but the tests will be ignored, and the execution will skip to the next sibling suite.

In detail:

• If before hook fails, tests in a suite (where the hook is) and sub-suites are not executed. If after hook exists, it is called.
• If before each hook fails, remaining tests in that suite (where the hook is) or sub-suites are not executed. The remaining before hooks are also not executed. If after each hooks exist in that suite, or parent suites -- they get executed. The suite's after hooks are also executed.
• If after hook fails, no additional hooks are called.
• If after each hook fails, remaining tests in that suite (where the hook is) and sub-suites are not executed. The remaining after hooks (from parent suites) are executed. After hooks are executed 'up' starting from the suite of the last executed test, finishing with a suite where the hook is (inclusively).

After any hook error, the next sibling suite (relative to the suite with hook error) is executed.

This allows writing test hooks in a symmetrical manner:

• before (each): allocate all resources for this and child test suites or tests
• after (each): free all resources, allocated by before (each). Make sure, this will work correctly, even if some resource allocation failed -- it will still be called.

[Rulikkk]

Any chances this is good/bad or no one needs this?

[lightsofapollo]

@Rulikkk ++

@visionmedia @travisjeffery Any chance this can get reviewed.... Its is a pretty big deal that failing hooks fail the entire test run.

[travisjeffery]

looks good. merged.

[lightsofapollo]

@travisjeffery big enough for a vbump? I would love to start using this ASAP.

[travisjeffery]

1.15.0's out now with this

[BryanDonovan]

This doesn't seem to work if the before/beforeEach is asynchronous. In the following test, the after() function is never called:

var assert = require('assert');

describe("before-and-after", function() {
    before(function(done) {
        process.nextTick(function() {
            assert.ok(false);
            done();
        });
    });

    after(function() {
        console.log("\nin after()");
    });

    it("the test", function() {
        assert.ok(true);
    });
});

Is it even possible to fix that? If so, should I file a separate bug?

[jest]

@BryanDonovan here's what should do it (I use it this to retrieve remote resources in before). However, if you expect the exception to be thrown also in nextTick, that should be wrapped in try-catch also.

var assert = require('assert');

describe("before-and-after", function() {
    before(function(done) {
        process.nextTick(function() {
            try {
                assert.ok(false);
            } catch (e) {
                done(e);
            }
            done();
        });
    });

    after(function() {
        console.log("\nin after()");
    });

    it("the test", function() {
        assert.ok(true);
    });
});```

[BryanDonovan]

Yeah, that would work, but it's very cumbersome to put a try/catch in every before() function.

[ajaks328]

I have a question regarding this fix. So, this issues seems to be resolved and is working as expected when using the hooks in sync mode. When I attempt to use the hooks in async mode (with done callback), sibling suites do not get executed.

If I have two sibling suites with async beforeEach, afterEach and tests - an async error thrown in the afterEach of the first suite will fail all tests in that suite (as expected) and also will stop execution of the second suite which does not seem expected.

Sample Code:

describe('Multiple Suites Test', function() {

describe('Suite 1', function() {

    beforeEach(function(done){
        console.log('Suite1:beforeEach');
        done();
    });

    afterEach(function(){
        console.log('Suite1:afterEach');

// Code to throw Error in async mode
// setTimeout(function(){
// throw new Error('Error in afterEach of Suite1!!');
// },1000);
throw new Error('Error in afterEach of Suite1!!');
});

    it('Suite1-Test1', function(done) {
        console.log('Suite1:Test1');
        done();
    });

    it('Suite1-Test2', function(done) {
        console.log('Suite1:Test2');
        done();
    });

});

describe('Suite 2', function() {

    beforeEach(function(done){
        console.log('Suite2:beforeEach');
        done();
    });

    afterEach(function(done){
        console.log('Suite2:afterEach');
        done();
    });

    it('Suite2-Test1', function(done) {
        console.log('Suite2:Test1');
        done();
    });

    it('Suite2-Test2', function(done) {
        console.log('Suite2:Test2');
        done();
    });

});

});

When the above file is executed you see the below output as expected:
Multiple Suites Test
Suite 1
Suite1:beforeEach
Suite1:Test1
✓ Suite1-Test1
Suite1:afterEach
1) "after each" hook
Suite 2
Suite2:beforeEach
Suite2:Test1
✓ Suite2-Test1
Suite2:afterEach
Suite2:beforeEach
Suite2:Test2
✓ Suite2-Test2
Suite2:afterEach

3 passing (14ms)
1 failing

1. Multiple Suites Test Suite 1 "after each" hook:
   Error: Error in afterEach of Suite1!!
   at Context. (simpleTests.js:12:19)

If the same code is run with commented code enabled and the first afterEach in async mode, the output looks like below,

Multiple Suites Test
Suite 1
Suite1:beforeEach
Suite1:Test1
✓ Suite1-Test1
Suite1:afterEach
1) "after each" hook

1 passing (1s)
1 failing

1. Multiple Suites Test Suite 1 "after each" hook:
   Uncaught Error: Error in afterEach of Suite1!!
   at null._onTimeout (simpleTests.js:14:23)

The second suite is completely ignored.

[ajaks328]

@BryanDonovan , I think your issue is similar to what I am seeing above. I have tried fixing this in a branch - ajaks328@65eba1c
You could try this fix and see if that works for you too.