Not forwarding upstream failures

[kgryte]

Consider the following failing test file

// test.js
throw new Error( 'beep' );

When run and piped to tap-xunit

$ node test.js | tap-xunit

the exit code for the pipeline is 0, as a non-zero error code when the process exits is not handled by tap-xunit.

To reproduce locally,

$ node -e 'throw new Error("beep");' | tap-xunit || echo 'Failed'

[eval]:1
throw new Error( "beep" );
^

Error: beep
    at [eval]:1:7
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:635:30)
    at evalScript (bootstrap_node.js:462:27)
    at startup (bootstrap_node.js:163:9)
    at bootstrap_node.js:608:3
<?xml version="1.0"?>
<testsuites/>

I would expect, similar to other tap reporters, such as tap-spec, that an upstream failure would propagate through tap-xunit. For an example of handling upstream exit statuses, see tap-spec.

$ node -e 'throw new Error( "beep" );' | tap-spec || echo 'failed'

[eval]:1
throw new Error( "beep" );
^

Error: beep
    at [eval]:1:7
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:635:30)
    at evalScript (bootstrap_node.js:462:27)
    at startup (bootstrap_node.js:163:9)
    at bootstrap_node.js:608:3


failed

For context, I am using tap-xunit on CircleCI. If upstream tests contain syntax errors, as opposed to test failures, the build succeeds, when, in fact, the build should fail. While I am aware of setting the pipefail option, I believe a better option would be for tap-xunit to account for upstream failures ensuring consistent behavior with other TAP reporters.

[kgryte]

Update: for context, tap-spec had a similar issue here. Their solution was to check for the plan line, as implemented in the relevant PR.

[aghassemi]

Thanks @kgryte. What I will do is exiting with 1 whenever there is a parse error. Fix coming soon.

[aghassemi]

Fix released under a new version 2.0.0 and published to npm already.
cheers

[aghassemi]

I think my fix only partially covers the issue. In case of upstream failures, stdout will be empty which I consider valid at the moment (as it does not generate parse errors with tap-parser). It should be fine to consider empty input invalid. Let me take another stab at it.

[aghassemi]

@kgryte 2.1.0 should fix this properly. Published already.

[kgryte]

@aghassemi Awesome! Thanks! Will take it for a spin now. :)

[kgryte]

@aghassemi So, the patch works for the case in the OP; however, not when TAP output is printed before the error message. To reproduce,

$ node -e 'console.log("TAP version 13\n# beep\nok 1 boop");throw new Error("ERROR");' | ./node_modules/.bin/tap-xunit || echo 'FAILED'

[eval]:1
console.log("TAP version 13\n# beep\nok 1 boop");throw new Error("ERROR");
                                                 ^

Error: ERROR
    at [eval]:1:56
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:635:30)
    at evalScript (bootstrap_node.js:462:27)
    at startup (bootstrap_node.js:163:9)
    at bootstrap_node.js:608:3
<?xml version="1.0"?>
<testsuites>
  <testsuite tests="1" failures="0" errors="0" name="beep">
    <testcase name="#1 boop"/>
  </testsuite>
</testsuites>

In this case, the TAP output is not valid, as it is missing the plan (which, in normal testing, may be absent due to a syntax error interrupting control flow before the plan is printed). But despite the TAP not being valid, tap-xunit still exits with exit code 0.

[aghassemi]

@kgryte fixed in 2.2.0. Sorry I had a bug in the test runner that did not catch that case although I had test for it.

[kgryte]

No worries! Thank you so much for your work and quick turnaround! Will test out now. :)

[kgryte]

I can confirm that my tests now fail in the event of an error thrown upstream of tap-xunit. Thanks again!

[aghassemi]

no problem, happy to help.

BTW, you can also pass --strict to tap-xunit which will use the strict mode of TapParser in which any non-TAP line in the input will throw. Default case is more relaxed and ignores non-TAP lines.