WIP: allow for pipes to drain

[kalinkrustev]

When running with -j, --output-file and --reporter options and when the child test output is big enough, some data may still be buffered and not yet passed to this.pipes. Firing the 'end' event will end the pipes, which will then fail because of unfinished parsing. Calling super.end, will properly check for pending drains.

[kalinkrustev]

@isaacs, is this the right direction to fix this bug? I tried several ways, though last commit seems to work best, except seems some tests may need adjustment.

[isaacs]

@kalinkrustev It would be best to start with a test case that shows the bad thing happening. Even a simple reproduction script would make it much easier to evaluate and provide direction here. Can you reproduce the problem?

I'm a little confused, because what you describe should be handled by code in other parts of the system. But without being able to see the effect directly, it's hard to comment.

[kalinkrustev]

Yes, you are right, test case will help a lot:

Given this:

// drain.js
const t = require('tap')

t.plan(5000);
for (let i = 1; i <= 5000; i++) {
    t.ok(true);
}

This passes:

tap drain.js

But this does not pass:

tap -odrain.txt drain.js

[isaacs]

Yeah, there's something weird going on here, for sure. It seems to only affect the root tap object.

Does this change the behavior for you?

const t = require('tap')

t.test('x', t => {
  t.plan(5000);
  for (let i = 1; i <= 5000; i++) {
      t.ok(true);
  }
})

[kalinkrustev]

Yes, it changes it. Now this is also passing:

tap -odrain.txt drain.js

But for example, this is not:

tap -odrain.txt -J drain.js

It has to do with the pipes, I traced it really hard and saw how the file stream related to the output file gets end()-ed (look here), while there is still buffered data not being sent to the file.

[isaacs]

I'm not saying you're wrong that that's (at least part of) the problem, and it's always possible that there are two bugs here. But this also fails for me, even when not using a -o or -J argument:

const t = require('tap')

t.plan(5000);
for (let i = 1; i <= 5000; i++) {
    t.pass(i);
}

Tap streams should be fully synchronous, so emitting end rather than calling Minipass.end should be fine. If that's fixing the problem, that's a clue, but the fact that it's causing other tests to break is making me think there's something deeper going on to make this possible. I'll dig into it tonight and see if I can uncover anything else.

At the very least, the failures in the promise and stdin tests need to be fixed before this can land.

[isaacs]

Please see #507. Thanks :)