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

tests should fail if before etc call back with an error or crash #103

Closed
garthk opened this issue May 30, 2014 · 6 comments
Assignees
Labels
bug
Milestone

Comments

@garthk
Copy link

@garthk garthk commented May 30, 2014

If before, beforeEach, after, or afterEach call back with an error, the test should fail.

Consider the following experiment:

var lab = require('lab');

lab.experiment('when \'before\' calls back with an error', function() {
    lab.before(function (done) {
        done(new Error('should fail'));
    });

    lab.test('the test runs and passes anyway', function (done) {
        console.error('oops');
        done();
    });
});

I'd expect that to log "Error: should fail", fail the test without running it, and exit with a non-zero code.

Under lab@3.2.1, though:

$ lab -v foo.js
should fail
Error: should fail
    at /private/tmp/foo/foo.js:5:14
    at Object._onImmediate (/private/tmp/foo/node_modules/lab/lib/execute.js:492:17)
    at processImmediate [as _immediateCallback] (timers.js:336:15)
oops
when 'before' calls back with an error
  ✔ 1) the test runs and passes anyway (3 ms)


 1 tests complete (8 ms)

 No global variable leaks detected.

$ echo $?
0

lab logs the error (good), runs the test anyway (bad), and exits cleanly (bad).

Similarly:

    lab.before(function (done) {
        throw new Error('should fail');
    });

… should fail tests, but doesn't.

@jaw187

This comment has been minimized.

Copy link

@jaw187 jaw187 commented May 30, 2014

Shouldn't there be a test which is dependent on the results from before or beforeEach?

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Jun 27, 2014

I agree with @garthk on this one, I've been very surprised it acted this way as well.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Jul 15, 2014

before and beforeEach are easy to do and the right thing, but how do you think after and afterEach to work? The tests already passed and the expectation is that after activities are cleanup.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Jul 15, 2014

I'd shamelessly inspire from mocha which seems to consider this as a test which won't appear in the reporting unless it fails (tests are named "after each" hook and "after all" hook).

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 3, 2014

This is partially fixed in 4.0.0. If before/beforeEach fail, the tests are not run. If after/afterEach fails, it is recorded and will cause the final code to be 1 (error). What is missing is to make the skipped tests (due to before failing) explicitly fail in the report.

@hueniverse hueniverse added the bug label Aug 3, 2014
@hueniverse hueniverse added this to the 4.0.0 milestone Aug 3, 2014
@hueniverse hueniverse self-assigned this Aug 3, 2014
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 3, 2014

Also, in 4.0 we no longer run before/after inside a domain which means if those fail by throwing, the whole thing just stops. We now consider anything outside of the tests to be application code which is the authors responsibility to manage. The before/after callbacks do take an error argument now used to indicate a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.