Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
test: update tap reporter version to 14#44041
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
anonrig commented Jul 29, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
MoLow commented Jul 29, 2022
How well does this play with tap parsers such as https://www.npmjs.com/package/tap-mocha-reporter ? |
anonrig commented Jul 29, 2022
Thanks for the question @MoLow. Here's the output using ➜ node git:(fix/tap-v14) ✗ ./out/Release/node ./tap-test.js | tap-mocha-reporter spec (node:66047) ExperimentalWarning: The test runner is an experimental feature. This feature could change at any time (Use `node --trace-warnings ...` to show where the warning was created) top-level test 1 ✓ level 1.1 ✓ top-level test 2 2 passing (5ms) |
manekinekko commented Jul 29, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@MoLow here is a screenshot comparing all 3 scenarios:
I think we are good! WDYT? |
MoLow commented Jul 29, 2022
Well only the first example looks correct to me. |
manekinekko commented Jul 29, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@anonrig specs 14 state that subtests need to be terminated by a test point. |
anonrig commented Jul 29, 2022
Actually this is related to Tap 14 and how tap-mocha-reporter does not handle tap 14 grammar correctly. If we want to support tap 14 we need to ignore this, since it is not backward compatible according to the specification Since TAP13 specifies that non-TAP output should be ignored or provided directly to the user, and indented Test Points and Plans are non-TAP according to TAP13, only the terminating correlated test point will be interpreted by most TAP13 Harnesses. Thus, they will usually report the overall subtest result correctly, even if they lack the details about the results of the subtest |
manekinekko commented Jul 29, 2022
Thanks for investigating this @anonrig! I agree we should ignore the mocha report case (unless there is a requirement I am not aware of). @MoLow the upcoming built-in TAP parser (#43525) is built on top of TAP 14. In most cases, TAP 14 is backward compatible with TAP13. I suggest we focus on TAP 14 and then later add TAP 13 specifics (if needed). |
aduh95 commented Jul 29, 2022
/cc @nodejs/test_runner |
MoLow commented Jul 30, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@manekinekko can the built-in parser support both TAP 13 and TAP 14? |
manekinekko commented Jul 31, 2022
I guess the answer is yes. The parser will need to implement both strategies. The current implementation will need some major refactoring but it's manageable. However, the current TAP 14 support is not 100% complete. I'd say we are 95%. Local pragmas support needs some twerking. Diagnostic data is parsed as raw data (not yaml). |
cjihrig commented Jul 31, 2022
Sorry for this unproductive comment, but I hope you meant to say tweaking 😄 |
anonrig commented Jul 31, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Should we produce different tap output according to different flags? What is the best alternative for producing 2 (or more) different tap versions for the test runner? |
manekinekko commented Aug 2, 2022
OMG!! Sorry about the typo. I definitely meant "tweaking" 😂 |
manekinekko commented Aug 2, 2022
According to specs, TAP 14 must be backwards compatible with TAP 13. |
anonrig commented Aug 2, 2022
I reverted my subtest print changes and just left the tap version print change and updated the tests. If there are any other changes required, I'll be happy to resolve @manekinekko |
ErickWendel commented Aug 8, 2022
@anonrig it seems some tests are broken now, would you mind checking it? |
anonrig commented Aug 8, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Updated the last commit and forced pushed with the unhandled test case pointing to version 13. @ErickWendel |
mattfysh commented Nov 26, 2022
@manekinekko apologies for the OTT - how are you getting |

Fixes#44040 according to TAP specification available on http://testanything.org/tap-version-14-specification.html
Example test:
Produced output: