Merge pull request #403 from nhamer/issue222

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

Fixes #222.

lib/test.js

@@ -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_);
@@ -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) {
@@ -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);
     }
@@ -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) {
@@ -298,9 +296,7 @@ Test.prototype._assert = function assert (ok, opts) {
         if (extra.exiting) {
             self._end();
         } else {
-            nextTick(function () {
-                self._end();
-            });
+            runProgeny.call(self);
         }
     }
 

test/add-subtest-async.js

@@ -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);
 })

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

test/nested-sync-noplan-noend.js

@@ -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');
     };
 
@@ -38,6 +45,7 @@ tap.test('nested sync test without plan or end', function (tt) {
                 q.end()
             }, 10);
         });
-    });
 
+        t.timeoutAfter(100);
+    });
 });

test/nested_test_ordering.js

@@ -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);
+    });
+});

test/subtest_and_async.js

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