Skip to content

Internal contradiction in the TAP version 13 spec #155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eric-s-raymond opened this issue Mar 3, 2021 · 7 comments
Closed

Internal contradiction in the TAP version 13 spec #155

eric-s-raymond opened this issue Mar 3, 2021 · 7 comments

Comments

@eric-s-raymond
Copy link
Contributor

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
Copy link
Member

mblayman commented Mar 3, 2021

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 (#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
Copy link
Contributor Author

@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.

@eric-s-raymond eric-s-raymond changed the title Internal contradiction in the TAP version 13 soec Internal contradiction in the TAP version 13 spec Mar 3, 2021
@mblayman
Copy link
Member

mblayman commented Mar 3, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

isaacs commented Mar 1, 2022

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants