Should a failed tearDown create an anonymous test?

[jtlapp]

tap-parser 2.0.0 breaks my tearDown() solution. Consider the following test:

var tap = require('tap');

tap.test("foobar test", function (t) {
    t.equal(123, 123);
    t.tearDown(function () {
        t.bailout("Oops, couldn't tear down foobar test");
    }); 
    t.end();
});

This test calls bailout during tear down. I know you weren't sure you wanted to support this, but how else should tap-parser handle a failure to tear down a test? A failed tear-down almost certainly compromises the remaining tests. I have forgotten why I came to the conclusion that the only way to know when a subtest completed was to give it a tear-down handler.

In any case, the above code results in the following tap-parser events, which place the "foobar test" bail-out in a new root-level "anonymous" test:

[
{'event':'version', 'data':13},

{'event':'child', 'data':"<childParser>"},

{'event':'comment', 'data':"# Subtest: foobar test\n"},

{'event':'assert', 'data':{"ok":true,"id":1,"name":"should be equal"}},

{'event':'plan', 'data':{"start":1,"end":1}},

{'event':'complete', 'data':{"ok":true,"count":1,"pass":1,"plan":{"start":1,"end":1},"failures":[]}},

{'event':'assert', 'data':{"ok":true,"id":1,"time":5.607,"name":"foobar test"}},

{'event':'child', 'data':"<childParser>"},

{'event':'comment', 'data':"# Subtest: (anonymous)\n"},

{'event':'complete', 'data':{"ok":false,"count":0,"pass":0,"bailout":"# Oops, couldn't tear down foobar test","failures":[]}},

{'event':'bailout', 'data':"# Oops, couldn't tear down foobar test"},

{'event':'bailout', 'data':"# Oops, couldn't tear down foobar test"},

{'event':'complete', 'data':{"ok":false,"count":1,"pass":1,"bailout":"# Oops, couldn't tear down foobar test","failures":[]}}
]

Assuming that you want to disallow bail-outs during tear-downs, I'll see if I can find another way to do this. (subtap has a feature for bombing out after the Nth failed subtest.)

[jtlapp]

I think it's a mistake to try to infer subtest boundaries. You may never surface from that rabbit hole. It's fine to have anonymous subtests, but I think tap has to explicitly demarcate them.

Have you tried writing an EBNF for TAP with subtests? Doing so would force you to face and resolve the ambiguities.

[jtlapp]

I believe I need tearDown() because I otherwise don't know when asynchronous tests have ended. I did successfully override Test::end() once upon a time, but that was some seriously fragile monkey-patching, and I'm not keen on doing it that way.

I really think it has to be possible for a generic clean-up function to terminate the test run.

[jtlapp]

I've temporarily resolved all my problems by adjusting the output of tap 7.0.0 to be compatible with tap-parser 1.2.2. I'm not sure I know how to upgrade to tap-parser 2.0.0.

[isaacs]

Upgrading to tap-parser 2 is easy. npm install tap-parser@latest --save

Seems like this patch would fix the issue? I agree that the output here is a little bit weird.

diff --git a/lib/test.js b/lib/test.js
index f1817f4..11a287e 100644
--- a/lib/test.js
+++ b/lib/test.js
@@ -1552,6 +1552,10 @@ Test.prototype.push = function (c, e) {
 }

 Test.prototype.bailout = function (message) {
+  if (this._parent && this._ended) {
+    this._parent.bailout(message)
+    return
+  }
   message = message ? ' # ' + ('' + message).trim() : ''
   message = message.replace(/^( #)+ /, ' # ')
   message = message.replace(/[\r\n]/g, ' ')

Script:

// test/test/teardown-bailout.js
var tap = require('../..');

tap.test('foobar test', function (t) {
  t.equal(123, 123)
  t.tearDown(function () {
    t.bailout('Oops, bad tear down')
  })
  t.end()
})

Before:

TAP version 13
# Subtest: foobar test
    ok 1 - should be equal
    1..1
ok 1 - foobar test # time=5.664ms

    Bail out! # Oops, bad tear down
Bail out! # Oops, bad tear down

After:

TAP version 13
# Subtest: foobar test
    ok 1 - should be equal
    1..1
ok 1 - foobar test # time=5.36ms

Bail out! # Oops, bad tear down

Here's how the parser interprets the indent:

$ node test/test/teardown-bailout.js | tap-parser -t
TAP version 13
    # Subtest: foobar test
    ok 1 - should be equal
    1..1
ok 1 - foobar test # time=5.323s
    # Subtest: (anonymous)
    Bail out!# Oops, bad tear down
Bail out!# Oops, bad tear down

[jtlapp]

Nice! Thank you sooooo much! I'll give it a shot.

Ha, I meant that I didn't know how to both upgrade my tool to tap-parser 2 and still have it function. But thank you for another demonstration of infinite patience!

I've been working on a cool plugin assertion for tap. It facilitates making easy-to-debug test suites for test runners as well as more generally comparing long text documents with minimal output to TAP -- while still accurately showing differences found between documents.

[isaacs]

I'm going to close this issue, because the parser is doing the right thing. It's node-tap that's producing weird output.