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

Fix premature end of tests (and running sibling tests) when test includes subtests #403

Merged
merged 3 commits into from
Jan 30, 2019
Merged
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
50 changes: 23 additions & 27 deletions lib/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@ var getTestArgs = function (name_, opts_, cb_) {
return { name: name, opts: opts, cb: cb };
};

var runProgeny = function () {
var self = this;
if (this._progeny.length) {
var t = this._progeny.shift();
t.on('end', function () { runProgeny.call(self) });
nextTick(function () {
t.run();
});
return;
}
if (this.calledEnd || this._plan) {
this._end();
}
};

function Test (name_, opts_, cb_) {
if (! (this instanceof Test)) {
return new Test(name_, opts_, cb_);
Expand Down Expand Up @@ -104,19 +119,11 @@ Test.prototype.test = function (name, opts, cb) {
this.emit('test', t);
t.on('prerun', function () {
self.assertCount++;
})
});

if (!self._pendingAsserts()) {
nextTick(function () {
self._end();
});
if (!this._pendingAsserts()) {
runProgeny.call(this);
}

nextTick(function() {
if (!self._plan && self.pendingCount == self._progeny.length) {
self._end();
}
});
};

Test.prototype.comment = function (msg) {
Expand All @@ -131,20 +138,19 @@ Test.prototype.plan = function (n) {
this.emit('plan', n);
};

Test.prototype.timeoutAfter = function(ms) {
Test.prototype.timeoutAfter = function (ms) {
if (!ms) throw new Error('timeoutAfter requires a timespan');
var self = this;
var timeout = safeSetTimeout(function() {
var timeout = safeSetTimeout(function () {
self.fail('test timed out after ' + ms + 'ms');
self.end();
}, ms);
this.once('end', function() {
this.once('end', function () {
safeClearTimeout(timeout);
});
}

Test.prototype.end = function (err) {
var self = this;
if (arguments.length >= 1 && !!err) {
this.ifError(err);
}
Expand All @@ -153,18 +159,10 @@ Test.prototype.end = function (err) {
this.fail('.end() called twice');
}
this.calledEnd = true;
this._end();
runProgeny.call(this);
};

Test.prototype._end = function (err) {
var self = this;
if (this._progeny.length) {
var t = this._progeny.shift();
t.on('end', function () { self._end() });
t.run();
return;
}

if (!this.ended) this.emit('end');
var pendingAsserts = this._pendingAsserts();
if (!this._planError && this._plan !== undefined && pendingAsserts) {
Expand Down Expand Up @@ -298,9 +296,7 @@ Test.prototype._assert = function assert (ok, opts) {
if (extra.exiting) {
self._end();
} else {
nextTick(function () {
self._end();
});
runProgeny.call(self);
}
}

Expand Down
9 changes: 5 additions & 4 deletions test/add-subtest-async.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ var test = require('../')
test('parent', function (t) {
t.pass('parent');
setTimeout(function () {
t.test('child', function (t) {
t.pass('child');
t.end();
t.test('child', function (st) {
st.pass('child');
st.end();
});
}, 100)
t.end();
}, 100);
})
21 changes: 21 additions & 0 deletions test/async_end.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
var test = require('../');

test('async end', function (t) {
setTimeout(function () {
t.assert(!t.ended, '!t.ended');
t.end();
}, 200);
});

test('async end with subtest', function (t) {
setTimeout(function () {
t.assert(!t.ended, '!t.ended');
t.end();
}, 200);

t.test('subtest', function (g) {
g.assert(!t.ended, 'subtest !t.ended');
g.end();
});
});

24 changes: 16 additions & 8 deletions test/nested-sync-noplan-noend.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,32 @@
var tape = require('../');
var tap = require('tap');
var concat = require('concat-stream');
var stripFullStack = require('./common').stripFullStack;

tap.test('nested sync test without plan or end', function (tt) {
tt.plan(1);

var test = tape.createHarness();
var tc = function (rows) {
tt.same(rows.toString('utf8'), [
tt.same(stripFullStack(rows.toString('utf8')), [
'TAP version 13',
'# nested without plan or end',
'not ok 1 test timed out after 100ms',
' ---',
' operator: fail',
' stack: |-',
' Error: test timed out after 100ms',
' [... stack stripped ...]',
' ...',
'# first',
'ok 1 should be truthy',
'# second',
'ok 2 should be truthy',
'# second',
'ok 3 should be truthy',
'',
'1..2',
'# tests 2',
'1..3',
'# tests 3',
'# pass 2',
'',
'# ok'
'# fail 1',
].join('\n') + '\n');
};

Expand All @@ -38,6 +45,7 @@ tap.test('nested sync test without plan or end', function (tt) {
q.end()
}, 10);
});
});

t.timeoutAfter(100);
});
});
98 changes: 98 additions & 0 deletions test/nested_test_ordering.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@

var tape = require('../');
var tap = require('tap');
var concat = require('concat-stream');

var stripFullStack = require('./common').stripFullStack;

tap.test('plan vs end: plan', function (tt) {
tt.plan(1);

var test = tape.createHarness();
test.createStream().pipe(concat(function (rows) {
tt.same(rows.toString('utf8'), [
'TAP version 13',
'# first',
'ok 1 first test',
'ok 2 t not ended',
'ok 3 t has progeny',
'# second',
'ok 4 second test',
'# third',
'ok 5 third test',
'',
'1..5',
'# tests 5',
'# pass 5',
'',
'# ok'
].join('\n') + '\n');
}));

test('first', function (t) {
t.plan(4);
setTimeout(function () {
t.ok(1, 'first test');
t.ok(!t.ended, 't not ended');
t.ok(t._progeny.length, 't has progeny');
}, 200);

t.test('second', function (t) {
t.plan(1);
t.ok(1, 'second test');
});
});

test('third', function (t) {
t.plan(1);
setTimeout(function () {
t.ok(1, 'third test');
}, 100);
});
});

tap.test('plan vs end: end', function (tt) {
tt.plan(1);

var test = tape.createHarness();
test.createStream().pipe(concat(function (rows) {
tt.same(rows.toString('utf8'), [
'TAP version 13',
'# first',
'ok 1 first test',
'ok 2 t not ended',
'ok 3 t has progeny',
'# second',
'ok 4 second test',
'# third',
'ok 5 third test',
'',
'1..5',
'# tests 5',
'# pass 5',
'',
'# ok'
].join('\n') + '\n');
}));

test('first', function (t) {
setTimeout(function () {
t.ok(1, 'first test');
t.ok(!t.ended, 't not ended');
t.ok(t._progeny.length, 't has progeny');
t.end();
}, 200);

t.test('second', function (t) {
t.ok(1, 'second test');
t.end();
});
});

test('third', function (t) {
setTimeout(function () {
t.ok(1, 'third test');
t.end();
}, 100);
});
});
18 changes: 10 additions & 8 deletions test/subtest_and_async.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@ var asyncFunction = function (callback) {
};

test('master test', function (t) {
t.test('subtest 1', function (t) {
t.pass('subtest 1 before async call');
t.test('subtest 1', function (st) {
st.pass('subtest 1 before async call');
asyncFunction(function () {
t.pass('subtest 1 in async callback');
t.end();
st.pass('subtest 1 in async callback');
st.end();
})
});

t.test('subtest 2', function (t) {
t.pass('subtest 2 before async call');
t.test('subtest 2', function (st) {
st.pass('subtest 2 before async call');
asyncFunction(function () {
t.pass('subtest 2 in async callback');
t.end();
st.pass('subtest 2 in async callback');
st.end();
})
});

t.end();
});