TestAnything / testanything.github.io

Test anything protocol website
testanything.org
Other
71 stars 64 forks source link

Internal contradiction in the TAP version 13 spec #155

Closed eric-s-raymond closed 2 years ago

eric-s-raymond commented 3 years ago

I'm writing a TAP consumer. While attempting to make it conform to the TAP version 13 spec I noticed a contradiction in the section on plan lines.

Second 'graf: "It must appear once, whether at the beginning or end of the output." But after the example, "The plan is optional but if there is a plan before the test points..."

So which is it?

mblayman commented 3 years ago

I think you've stumbled upon one of the many inconsistencies within the TAP spec. I suspect inconsistencies like this are largely the motivation for the TAP version 14 draft spec (https://github.com/TestAnything/testanything.github.io/pull/36). Unfortunately, that draft stalled out many years ago.

As the author of tappy, I decided to interpret that part of the spec to mean that the plan is optional before the TAP stream, but then must be included later. That's not strictly what the spec says, but I made the call to be a bit more stringent for my sanity. https://github.com/python-tap/tappy/blob/0c38a487d6e0113412902ab7a521120cf9da332f/tap/rules.py#L28-L32 is the code I wrote to handle this.

In the end, this part of the spec is a bit of a choose your own adventure. That's not exactly a good quality for a specification. 😄 I'm not sure what choices the other implementers made.

eric-s-raymond commented 3 years ago

@mblayman: Thanks, it's nice to know I'm not seeing things.

Also good to make contact with the author of tappy, which is the consumer I'm actually using for the test suite of this project: https://gitlab.com/esr/reposurgeon

reposurgeon has 449 tests, and I've TAPified it this last week because I had a few too many recent instances of pushing bad commits when the message that should have told me a test was broken was scrolled offscreen by success messages. My TAP producers are a bunch of custom test scripts; my consumer is tappy.

Not for long, though. You did good work but it's unpleasantly heavyweight for my use case, which is better served by a much smaller and simple standalone TAP consumer that doesn't call libraries or pull in a lot of producer-oriented complexity I'm not using - something lightweight enough that I can just commit a single file into my test directory and be done. So yesterday I wrote this: https://gitlab.com/esr/tapview

It looks and works a lot like tappy but it's written as 172 lines of POSIX shell - no external dependencies whatsoever except that your /bin/echo has to support -n, which POSIX allows but does not require. Soon I will replace my tappy deployment with tapview, and post a PR to the TAP website adding it to the consumer listing eight next to tappy. Going to let the code cool down a bit and see if it attracts any issue reports before I do the latter.

I posted this bug because I wanted to get tapview fully conformant to the spec. Perhaps there are other inconsistencies, but this is the only one that jumped up and demanded my attention. In the event, I chose the same interpretation you did.

There is one other point I'm still unclear on. What is best practice for when a viewer should dump a detail section? After not-ok lines? After either ok or not-ok lines? I'm not sure what tappy's rules are, nor what tapview's should be.

mblayman commented 3 years ago

I'm glad tappy was able to serve your project, even if only for a short time.

I don't know exactly what kind of "detail section" you have in mind because such a concept doesn't exist in the spec by that name; however, my stance was to dump out any information that tappy received as diagnostics immediately. My thought process was that I wanted spatial locality of output.

For instance, if pytest catches an error exception, I wanted to dump it out immediately after producing a not ok line so that users would know why. I thought that separating the failure message from the details about the failure would be confusing.

I hope that helps.

eric-s-raymond commented 3 years ago

I thought "detail section" was the term for a block of one-space-indented information following a status line.

The way tapview works is like tappy - the dot display happens first, then "no ok" lines saved up during the run are dumped. What I'm not clear on is under what circumstances following diagnostics (whether plain text otr YAML) should be passed through to the human viewer - if there is any normative guidance anywhere about this I have not found it.

eric-s-raymond commented 3 years ago

1OK, I've gone back and reread the spec carefully and think I see where I misunderstood it. Forget all my rambling about detail sections.

isaacs commented 2 years ago

Sorry, late to this thread, but just wanted to say yes, node-tap and I believe the modules in CPAN do the same as tappy in this regard.

A plan is required, but it can be either at the beginning or the end. If there is a plan at the beginning, then that is "the plan", and the trailing plan is "invalid TAP" (meaning parsers/reporters can ignore it, pass it through as-is to stdout, throw an error and crash, or report it as unrecognized random output in some other way).

So in most parsers, this is ok:

TAP version 13
1..2
ok
ok
1..2 # <-- unrecognized random stdout garbage noise

But this is not ok:

TAP version 13
1..1
ok
ok # <-- invalid tap, test count exceeds plan, probably a test failure
1..2 # <-- noise
mblayman commented 2 years ago

Version 14 is nearly wrapped up and tries to sharpen up language on subject like this. I'm going to close this one. https://github.com/TestAnything/Specification/pull/25