Tap14 - Round 2, seven years in the making #25
Conversation
The goal this time around will be to make small uncontroversial
changes, which can step us into paving the paths laid by node-tap,
Test::More, tappy, and many others over the years. There will be no
innovation here, just cataloging what exists.
Goals:
* No changes will be accepted UNLESS they are already widely adopted by
multiple implementations representing a majority of the TAP userbase.
* No changes will be blocked IF they are already widely adopted by
multiple implementations representing a majority of the TAP userbase.
* Specification should remove as much ambiguity as possible, while
maintaining backwards compatibility with existing TAP providers and
consumers.
Features/Changes Anticipated:
* Change the version from 13 to 14 (of course)
* Require that yaml diagnostic blocks be actual yaml, stipulate that
they are to be indented 2 spaces.
* Specify child tests (both indented and {}-wrapped buffered), indented
4 spaces.
* Specify pragmas (at least, define the syntax, even if the specific
keys are left up to implementations)
* Clean up and clarify ambiguous language.
Hopefully this much we can agree upon, at least ;) Note that a parser is still technically allowed to interpret a `TAP version 13` stream as TAP14 and be compliant with this spec, but this behavior is not required. (In other words, they can also throw that out as an error, and also be compliant.) Since the goal is to codify existing behavior and add little to it, and all current "TAP13" implementations allow `TAP version 13`, this follows logically.
- Provide rough EBNF grammar for the tap document. - Allow Harnesses to accept `TAP version 13` and still be TAP14 compliant. - Group all Test Point definitions together. - Include RFC2119 language note. - Clarify Harness behavior without being implicitly tied to perl command line test runners. - Clarification of TODO and SKIP directives, specify that they MUST NOT be treated as a test failure, even if the Test Point is `not ok`. - Specify that YAML blocks must be indented by 2 spaces. - Provide guidance around test exit status, handling of invalid TAP lines, and other Harness behavior.
|
At this point, basically every current TAP13 harness should also be a valid TAP14 harness according to this spec, and with the exception of the Version line, every TAP14 document is also a valid TAP13 document (though not vice versa, owing to additional strictness in the YAML indentation). If we can accept this much and move forward, the remaining work is just to specify how child tests are defined. Which is the biggest (really, only) feature we're talking about adding for TAP14, and since there's already fairly broad consensus on what we support, I hope it should be straightforward. |
These goals sound sensible to me. |
There was a problem hiding this comment.
Great work @isaacs , thanks a lot for writing this! +1 to adding your name under authors as suggested in another comment.
I added a lot of comments, but as a non native speaker, some may be irrelevant. Other comments were about differences between TAP13 and TAP14, or about the grammar.
But this version looks really promising! Once the comments are resolved, I'll hit approve and prepare to update the Java implementation tap4j and the Jenkins plug-in to support it too. I think TAP14 wouldn't break anything in tap4j, but pragmas are ignored as invalid line, so I'll just change it to parse them correctly.
Great work again, bravo 👏
-Bruno
|
Thanks for all the feedback, I'll get some time to take another pass through this either today or this weekend. |
Co-authored-by: Matt Layman <[email protected]>
|
TODO:
|
| and any text after `TODO\S*\s+` is the explanation. | ||
|
|
||
| ```tap | ||
| not ok 14 # TODO bend space and time |
There was a problem hiding this comment.
Should the implementations note (prefix) # TODO tests as not ok in any case?
|
First draft of subtests are in. PTAL when you get the chance. Thanks! |
There was a problem hiding this comment.
The definition of subtests that you added seems to match up with the discussion. I think it provides enough specificity to meaningfully describe subtests.
As someone with an implementation that doesn't handle subtests, I appreciate the detail and examples provided here.
👍
Co-authored-by: Matt Layman <[email protected]>
|
Haven't had any other input, and as far as I can tell, this is just about complete. Unless there's any objection, let's call it done 2 weeks from now? (ie, Thursday, April 14.) |
|
I like that plan. That's a reasonable amount of time to work through any final comments that might pop up. |
|
Alright, the deadline came and no one raised any new issues. I'll clean up the commit history and we can make it official. |
|
Can I get a 👍 on TestAnything/testanything.github.io#164 to add to the website? |
| TAP version 13 | ||
| ``` | ||
|
|
||
| Harnesses _may_ treat any TAP stream lacking a version as a failed test. |
There was a problem hiding this comment.
I implemented this in Meson as a passing test which verbosely warns you not to do this. I got a response:
mesonbuild/meson#11186 (comment)
Meson is able to parse TAP12 and TAP13, therefore it does not have to warn if the version line is missing.
IMO the "Harnesses may treat any TAP stream lacking a version as a failed test" is just silly. The point of TAP is exactly that it is easy to produce without relying on a library that handles the versioning and formatting for you, so good luck enforcing that with dozens of TAP producers.
Thoughts?
There was a problem hiding this comment.
More importantly:
In fact the test-anything.org page says:
Here’s what a TAP test stream looks like:
1..4 ok 1 - Input file opened not ok 2 - First line of the input valid ok 3 - Read the rest of the file not ok 4 - Summarized correctly # TODO Not written yet
Which regardless of anything else seems to be a problem -- the homepage of https://testanything.org/ describes a TAP stream without the soft-mandatory version, so it is valid TAP 12 but a TAP 14 harness can report it as a test failure. Maybe this should include a version...
There was a problem hiding this comment.
Thanks for bringing it up here @eli-schwartz. In my opinion the correct reading of the spec is:
-
a TAP harness MUST treat a missing version line as version 12
-
a TAP harness MAY reject versions below any limit they would like to apply (even though it's not a good idea :))
The latter does imply rejecting input without a version line (which puts the limit at version 13), but does not mean that harnesses should warn if the version line is absent.
There was a problem hiding this comment.
Let's discuss this in a new issue, I think it's a useful subject to dig into and get more opinions on.
#41
The goal this time around will be to make small uncontroversial changes, which can step us into paving the paths laid by node-tap, Test::More, tappy, and many others over the years. There will be no innovation here, just cataloging what exists.
Goals:
No changes will be accepted UNLESS they are already widely adopted by multiple implementations representing a majority of the TAP userbase.
No changes will be blocked IF they are already widely adopted by multiple implementations representing a majority of the TAP userbase.
Specification should remove as much ambiguity as possible, while maintaining backwards compatibility with existing TAP providers and consumers.
Features/Changes Anticipated: