Repository navigation
test_runner: update output TAP format to follow TAP 14 specs #44040
Description
What steps will reproduce the bug?
- create an
index.jsfile with a few test cases:
const test = require('node:test');
const assert = require('node:assert');
test('top-level test 1', async (t) => {
await t.test('level 1.1', () => {});
});
test('top-level test 2', () => {});- run the test:
node index.js(note:node --test index.jsgives an incorrect result)
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior?
Even though the current output is syntactically valid TAP output. The expected output would be a TAP format that follows specs 14.
TAP version 14
# Subtest: top-level test 1
ok 1 - sub test level 1.1
1..1
ok 1 - top-level test 1
ok 2 - top-level test 2
1..2
Note the version header TAP version 14 (required by TAP14).
What do you see instead?
We currently output the following format (removed diagnostics for readability):
TAP version 13
# Subtest: top-level test 1
# Subtest: sub-test level 1.1
ok 1 - sub-test level 1.1
1..1
ok 1 - top-level test 1
1..1
Top-level tests are incorrectly output as subtests: # Subtest: top-level test 1 and # Subtest: top-level test 2
Additional information
As a reference, using node-tap (v16.3.0) gives the following output:
TAP version 13
# Subtest: top-level test 1
ok 1 - sub test level 1.1
1..1
ok 1 - top-level test 1
ok 2 - top-level test 2
1..2
Here is the diff:
TAP version 13
# Subtest: top-level test 1
- # Subtest: sub test level 1.1
ok 1 - sub test level 1.1
1..1
ok 1 - top-level test 1
+ ok 2 - top-level test 2
-1..1
+1..2
Activity
manekinekko commentedon Jul 29, 2022
cc @benjamingr @cjihrig @MoLow
anonrig commentedon Jul 29, 2022
I've given this task a try, and opened a pull request. Happy to solve any issues I've missed or is introduced.
manekinekko commentedon Jul 29, 2022
Thanks @anonrig 👍
aduh95 commentedon Jul 29, 2022
/cc @nodejs/test_runner
ljharb commentedon Oct 15, 2022
Why is this not planned?
MoLow commentedon Oct 15, 2022
The current output is a valid TAP output and currently more parsers support it better than the proposed changed.
Since @anonrig closed his PR, I thought the issue can be closed
ljharb commentedon Oct 15, 2022
Given that TAP 14 added better support for nested tests, it seems like a pretty critical thing to add. I agree outputting TAP 13 is a better default, but it still seems like something node should be allowing via an option, if it wants to be in the test runner business.
MoLow commentedon Oct 16, 2022
I amm reopening - but I think this should only be changed once there is an appropriate support for this change from tap parsers/reporters
manekinekko commentedon Oct 16, 2022
FYI, the TAP parser from #43525 was designed initially based in TAP14. I have made changes to allow it to parse TAP13 (mainly for subtests). So I guess it should be able to handle valid TAP14 content.
anonrig commentedon Oct 16, 2022
IMHO Adding --tap-version=14 to test runner would resolve this issue. I’m changing countries in a couple of days, and can look into this issue a week from now.
daltonna commentedon Nov 10, 2022
Can someone assign this issue to me?
mattfysh commentedon Nov 26, 2022
Is this why userland reporters like
tap-arcare not reporting any useful information?i.e.
node --test | tap-arcdoesn't show any diff information and also saysoperator: undefinedcjihrig commentedon Nov 26, 2022
No. The output of the CLI runner should be nicer in the next release (v19.2.0) due to #43525 being merged. For example:
The
operator: undefinedthing is specific totap-arc, and it looks like they have a WIP PR to better supportnode:testoutput. Node also landed e260b37 recently, which may help out here.eyitayoit-alt commentedon May 21, 2023
I see the following output
TAP version 13
Subtest: top-level test 1
ok 1 - top-level test 1
duration_ms: 0.0321595
...
Subtest: top-level test 2
ok 2 - top-level test 2
duration_ms: 0.0005513
...
MoLow commentedon May 22, 2023
I am closing this now as I don't think an issue is needed.
if someone opens a pr to change this it is fine - we don't parse TAP anymore so it is also less fragile