Fix comment order

[isaacs]

Keep a queue of comments that occur when we're waiting on potential
diags for a previous assert.

Then emit those comment events all at once when we are sure that either
the test point line is finished (without diags) or that the diags are
done.

Fix #18
Fix #19
Fix #20

r? @simov @anko

[isaacs]

Travis failures are because of nodejs/node#1854

[anko]

Works as expected. Many thanks! 👍✨

[simov]

@isaacs I was working under the assumption that the yamlish diag follows directly after the assert line. The additional tests change the logic a little bit I agree. Also I was fixing the assert event position, rather than the comment one, but if that doesn't cause any problems I'm fine with not having it fixed. I'll make a few more tests these days and I'll let you know if something doesn't work for me.

[isaacs]

@simov They do, except that comments and blank lines can occur anywhere within a TAP stream, which is why the comment originally appeared before the assert. We don't yet know that the assert is finished, but we do know that the comment is done, so my thinking was, go ahead and emit it now. (Blank lines aren't a problem because we just ignore those.)

Note that 'extra' events still will come before the assert output as of this patch, so this might still be weird:

TAP version 13
1..1
blearggy blags
not ok 1 - weirdness
again with the weirds

That parses as:

[ [ 'version', '13' ],
  [ 'plan', { start: 1, end: 1 } ],
  [ 'extra', 'blearggy blags\n' ],
  [ 'extra', 'again with the weirds\n' ],
  [ 'assert', { ok: false, id: 1, name: 'weirdness' } ],
  [ 'complete',
    { ok: false,
      count: 1,
      pass: 0,
      fail: 1,
      plan: { start: 1, end: 1 } } ] ]

We can of course just buffer 'extra' lines like we do comments, but since those are technically parse errors anyway, maybe it's not as big a deal?

[simov]

@isaacs I was probably misguided by the fact that the line events inside the test fixtures are not immediately followed by they respective comment or assert event. Anyway I guess they are used only internally.

As for the extra events my suggestion is that we should buffer them as well.

A small quote from the spec:

Anything else
Any output line that is not a version, a plan, a test line, a diagnostic or a bail out is considered an "unknown" line. A TAP parser is required to not consider an unknown line as an error but may optionally choose to capture said line and hand it to the test harness, which may have custom behavior attached. This is to allow for forward compatability. Test::Harness silently ignores incorrect lines, but will become more stringent in the future. TAP::Harness reports TAP syntax errors at the end of a test run.

In my understanding the extra event is used for extending the protocol. For example a tap reporter may rely on custom data.

If I put the following code on top of each of my test files, I can then listen for extra events matching some pattern. In my case I would like to know when the new file starts, and I don't want to rely on comment event for that.

var it = require('tape')

it('file test1.js', function (t) {
  console.log('file test1.js')
  t.end()
})