Failing test in basic.js

[Jameskmonger]

A test in basic.js appears to be failing, causing the Travis build to fail.

test/basic.js ......................................... 5/6
  not ok test/basic.js
    arguments:
      - test/basic.js
    timeout: 30000
    results:
      count: 2
      plan:
        start: 1
        end: 3
      pass: 2
      ok: false
    command: 'C:\Program Files\nodejs\node.exe'
    file: test/basic.js

The above is from npm test (or tap test/basic.js), but when I run node test/basic.js I get the following output:

λ node test\basic.js
TAP version 13
ok 1 - calling as function returns instance
    # Subtest: passing no options and cb works fine
    1..0
ok 2 - passing no options and cb works fine # time=6.58ms

    # Subtest: takes a buffer just fine
    ok 1 - called cb from normal write
    ok 2 - called cb from post-bailout write
    ok 3 - should match pattern provided
    1..3
ok 3 - takes a buffer just fine # time=15.09ms

1..3
# time=48.484ms

[Jameskmonger]

@isaacs Any idea what might be causing this?

[isaacs]

@Jameskmonger I can't get this to fail locally, but it's really weird. It's like it's getting that there's a plan of 1..3 but only seeing 2 of the tests. Since the plan is the second to last line, after all the test blocks, it can't be that it's truncating the output.

If you have it reliably failing locally, can you try running this?

npm test -- -Rtap -b

That should post the output as raw TAP so we can see what the runner is getting from the child process.

[evanlucas]

I think I'm seeing this locally in my own project as well. It seems like tap doesn't like nested tests anymore? fwiw, npm install tap-parser@1.2 fixes the issues for me

[isaacs]

@evanlucas: What version of tap are you using? npm ls tap

My hunch is that that's a different issue, but could be related.

[evanlucas]

6.3.0 (tap-parser@1.3.1) and sorry to hijack this issue :]

[isaacs]

@evanlucas: It's fine :) Do you have a link to the tests? What version of node/os/etc?

[evanlucas]

Here is a reduced test case:

It seems to only happen when you have child tests and the parent uses t.plan() if that helps

'use strict'

const test = require('tap').test

test('top level test', (t) => {
  t.plan(1)
  t.test('test 1', (tt) => {
    tt.plan(1)
    setTimeout(() => {
      tt.pass('test passed')
    }, 1000)
  })
})

Node: v6.3.1
OS: Mac OS X 10.11.6

[isaacs]

Ah, i can reproduce on Linux. Will dig into this :)

[evanlucas]

cool, thanks!

[isaacs]

It's definitely a regression in the parser that came about as a result of adding support for non-indented # Subtest: <name> directives. But it only seems to happen on some systems, so I wonder if it's some kind of timing issue.

For now, I tagged tap-parser@1.2.2 as latest, so it'll stop biting people. I'll hopefully have a fix soon-ish. Unindented Subtest directives are a lot better to work with, and it's more popular, so this module should parse them properly (but without regressing).

[evanlucas]

thanks @isaacs!

[isaacs]

Ok, I see the problem.

The parser as of 1.3.0 allows for a not-indented # Subtest: <name> directive, and Test2 "buffered" style subtests.

The problem is that I was treating the indented subtest directives and not-indented subtest directives using the same logic. So, when tap "helpfully" emits a data event on the child proc's stdout, the child test's parser read that as a sub-subtest, and it mucked up all the logics.

The mystery is why it works on my OSX machine.

So, supporting both indented and not-indented subtest directives is going to require a bit more work, and this is clearly a breaking change to the parser, since it requires code changes in node-tap as well.

[rmg]

For now, I tagged tap-parser@1.2.2 as latest, so it'll stop biting people.

Unfortunately, tap depends on tap-parser@^1.2.2 so it's still pulling in tap-parser@1.3.1 :-(

Now I've got some tests that work when you do node some-test.js or node some-test.js | tap but fail if you run tap some-test.js. It's very weird behaviour! I spent 2 hours trying to debug it before remembering node-tap switched to using this module (boo!), and then about 10 seconds to find this issue (yay!).

[isaacs]

Things should be working fine again, after some rather kludgey publishes and orphaned-branch git reworking.

The "move forward" solution will be a major version bump, making it so that the Subtest comment directives aren't strictly required at all, so it'll work no matter where they are (or if they're omitted).