Skip to content

Commit

Permalink
Fix issue222
Browse files Browse the repository at this point in the history
There are two bugs here, one a matter of correctness, the other a matter of changing behavior between plan and end

(1) without plan(), the parent test would be _ended prematurely as soon as all queued subtests completed, and
sibling tests of the parent would run

(2) plan() runs all assertions in the plan, then starts running subtests. end() ran subtests on nextTick after creation
(see Test.prototype.test), so the relative ordering of asserts vs subtests could change with async asserts.
New behavior without a plan is to only start subtests after t.end().
  • Loading branch information
nhamer authored and ljharb committed Oct 15, 2017
1 parent c63b15a commit 45a5066
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 24 deletions.
44 changes: 20 additions & 24 deletions lib/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,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()) {
this._runProgeny();
}

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

Test.prototype.comment = function (msg) {
Expand Down Expand Up @@ -144,7 +136,6 @@ Test.prototype.timeoutAfter = function(ms) {
}

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

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 All @@ -177,6 +160,21 @@ Test.prototype._end = function (err) {
this.ended = true;
};

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

Test.prototype._exit = function () {
if (this._plan !== undefined &&
!this._planError && this.assertCount !== this._plan) {
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();
});
self._runProgeny();
}
}

Expand Down
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();
});
});

0 comments on commit 45a5066

Please sign in to comment.