test_runner: update output TAP format to follow TAP 14 specs

[manekinekko]

What steps will reproduce the bug?

• create an index.js file 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.js gives 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

Related: #43417 #43525

[manekinekko]

cc @benjamingr @cjihrig @MoLow

[anonrig]

I've given this task a try, and opened a pull request. Happy to solve any issues I've missed or is introduced.

[manekinekko]

Thanks @anonrig 👍

[aduh95]

/cc @nodejs/test_runner

[ljharb]

Why is this not planned?

[MoLow]

Why is this not planned?

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]

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]

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]

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]

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]

Can someone assign this issue to me?

[mattfysh]

Is this why userland reporters like tap-arc are not reporting any useful information?

i.e. node --test | tap-arc doesn't show any diff information and also says operator: undefined

  Subtest: /sandbox/test_runner/main.js
    ✖ 1) /sandbox/test_runner/main.js
      operator: undefined

[cjihrig]

Is this why userland reporters like tap-arc are not reporting any useful information?

No. The output of the CLI runner should be nicer in the next release (v19.2.0) due to #43525 being merged. For example:

$ ./node --test test.js | npx tap-arc
    ✖ 1) spies on async class static functions
      Expected to throw
      
    ✔ spies on async class functions

  cancelled 0

  duration_ms 95.023375

  Failed tests: There was 1 failure

    ✖ 1) /Users/cjihrig/iojs/node/test.js

  total:     1
  failing:   1
  34 ms

The operator: undefined thing is specific to tap-arc, and it looks like they have a WIP PR to better support node:test output. Node also landed e260b37 recently, which may help out here.

[eyitayoit-alt]

I see the following output
TAP version 13

Subtest: top-level test 1

# Subtest: level 1.1
ok 1 - level 1.1
  ---
  duration_ms: 0.0214175
  ...
1..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]

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