Stop createStream for creating additional test runner loops

[billti]

Without this change, every call to createStream starts an additional loop of test execution in the Results instance. Only the first stream creation should start test execution.

This was tricky to track down, but in my scenario it was causing the self.emit('done') in the loop to fire early (as the additional loop would find no tests waiting, while the prior loop was still executing the last test), which kills the stream (due to the output.queue(null) in the done handler), which meant the detection of unfinished tests on the process.on('exit'... handler couldn't write to the stream.

The below simple repro shows the problem. Without this change the console.log in the stream event handler will never fire. With this change, it does.

I couldn't figure out how to turn this into a test case, as it depends on the handling of the process exit event. (As noted below, even adding another handler for this doesn't work, as Tape stop the process from running additional handlers in its exit handler).

This was found working on improvements to the Tape test running in Visual Studio, which depends on an object stream to report results (microsoft/nodejstools#1524).

var tape = require('tape');

var harness = tape.getHarness();
var objStream = harness.createStream({objectMode: true});

objStream.on('data', function(evt){
    if (evt.type === "assert" && evt.name === "test exited without ending") {
        console.log("Successfully got the test failure events");
    }
});

tape('is ok', function(t){
    t.ok(true, 'value is true');
    t.end();
});

tape('no end', function(t){
    t.ok(true, 'another sucessful test!');
    // oops... didn't end.
});

process.on('exit', function(code){
    // Unfortunately can't check here, as this will never run.
    // The process.on('exit'...) handler in tape's createExitHarness method
    // calls process.exit(), which causes Node.js to exit immediately, and
    // not run further 'exit' event handlers (not sure if this is by design,
    // the Node.js docs don't mention this behavior).
});

[ljharb]

Thanks - could you add a regression test that would have prevented this?

[billti]

It took over a year and a half just to get a response. I don't even remember what this is now.

[ljharb]

Apologies that sometimes things slip through the cracks.

Your OP seems to have lots of context; it’d be a shame to lose this fix if there’s a way we can get a regression test out of it.

[billti]

Looks like the linked #404 (mentioned above) fixes the same issue and has a test.

[ljharb]

Ah, thanks! I’ll try to incorporate these two together then.