search

scottcorgan / tap-out

A different tap parser
MIT License
23 stars 28 forks source link

Question: How can errors in tap output be represented in tap-out? #26

Closed matthewdunsdon closed 7 years ago

matthewdunsdon commented 7 years ago

There are several cases when all the tests pass, however the overall tap input indicates a problem.

Could the output of tap-out include an errors section in the response and / or introduce an 'error' event?

Context

The main case I am seeing is when test file dies silently in the middle of its run. When this happens you get the following output from tap.

ok 1
ok 2
ok 3
ok 4

This output should be treated as an error because the test died before plan section is written (i.e. 1..5). The output from tap-out@1.4.2 is included on one of my gists.

To work around this in the short term, I could verify that the plans property from tap-out is not an empty array — technically plans property is not documented in README.md.

Because there are more error cases that can occur, such as having more tests run than are initially planned, it could be beneficial for consumers to opt into reading this new data or listening to an additional event rather than detecting these errors themselves.

scottcorgan commented 7 years ago

Good find. I'm open to a PR.

Also, could you test, the module that was meant to replace this one is https://github.com/tap-format/parser.

matthewdunsdon commented 7 years ago

We have a PR created for tap-out, which should hopefully be a place for discussion what the preferred mechanism is for outputting errors.

Next action: take a look into https://github.com/tap-format/parser.

Question: Is the intention that the tap-out will be be deprecated and that @tap-format/parser is the recommended library to use instead?

If tap-out is not being maintained anymore and you are just using a different library yourself, then a deprecation notice on the README file and doing an npm deprecate could be helpful to the community.

scottcorgan commented 7 years ago

Thanks for the PR! I'll review the PR in a bit.

As far as deprecating this package, I don't think I'm going to deprecate it. It seems to be popular and has held up well over the years.

scottcorgan commented 7 years ago

Closed by https://github.com/scottcorgan/tap-out/pull/27

matthewdunsdon commented 7 years ago

Have taken a look at https://github.com/tap-format/parser, and there are similar issues there as well.

Because it is event / observer based, I am not sure if a different approach is needed. I think it might be worth doing, as it allow consumers to listen to errors rather than having to verify that no plan events has been emitted.