From 45a506679d5e1b9345fdb339c813f141b1064f0d Mon Sep 17 00:00:00 2001 From: Norman Date: Sun, 15 Oct 2017 10:09:03 -0700 Subject: [PATCH] Fix issue222 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(). --- lib/test.js | 44 ++++++++++++++++++++------------------------ test/async_end.js | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 24 deletions(-) create mode 100644 test/async_end.js diff --git a/lib/test.js b/lib/test.js index 127fe4ef..a7f18397 100644 --- a/lib/test.js +++ b/lib/test.js @@ -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) { @@ -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); } @@ -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) { @@ -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) { @@ -298,9 +296,7 @@ Test.prototype._assert = function assert (ok, opts) { if (extra.exiting) { self._end(); } else { - nextTick(function () { - self._end(); - }); + self._runProgeny(); } } diff --git a/test/async_end.js b/test/async_end.js new file mode 100644 index 00000000..a5239a43 --- /dev/null +++ b/test/async_end.js @@ -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(); + }); +}); +