Only print failing assertion once with spec reporter

[legendecas]

What is the problem this feature will solve?

The default test runner reporter spec prints an error twice. For example, with the following simple test:

const test = require('node:test');

test('foo', () => {
  throw new Error('foo');
});

The command node --test test.js, or node test.js prints:

✖ foo (0.308375ms)
  Error: foo
      at TestContext.<anonymous> (/Users/cwu631/Developer/nodejs/node/test.js:4:9)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:931:25)
      at Test.start (node:internal/test_runner/test:829:17)
      at startSubtestAfterBootstrap (node:internal/test_runner/harness:297:17)

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 50.714375

✖ failing tests:

test at t.js:3:1
✖ foo (0.308375ms)
  Error: foo
      at TestContext.<anonymous> (/Users/cwu631/Developer/nodejs/node/test.js:4:9)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:931:25)
      at Test.start (node:internal/test_runner/test:829:17)
      at startSubtestAfterBootstrap (node:internal/test_runner/harness:297:17

A single test caused two identical errors in the output.

When there are 2 or more test failures, the output is really verbose to be inspected by a human -- are these the same failures?

This hurts the experience with node:test in node core development. A change can fail many cases in the test, and every test failure is printed twice, causing super long outputs.

What is the feature you are proposing to solve the problem?

Only print a single test identical failure detail exactly once. This reduces the length of the output on failure and allow people to focus, rather than been distracted by the verbose long output.

✖ foo (0.308375ms)
  Error: foo
      at TestContext.<anonymous> (/Users/cwu631/Developer/nodejs/node/test.js:4:9)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:931:25)
      at Test.start (node:internal/test_runner/test:829:17)
      at startSubtestAfterBootstrap (node:internal/test_runner/harness:297:17)

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 50.714375

✖ failing tests:

test at t.js:3:1
✖ foo (0.308375ms)

What alternatives have you considered?

Don't change the status quo.

[pmarchini]

Hey @legendecas,
I completely agree with you.
I was thinking the same a few days ago while reviewing some tests output.

I’ll take a look if nobody else gets to it before me

[cjihrig]

Definitely +1 to only printing the failure once. Which one will you keep?

For context, the list at the end was added because people complained about scrolling back to see failures. I would see which approach other runners take and do what seems to be most popular.

This hurts the experience with node:test in node core development.

FWIW, I think we should make this particular change, but I don't think we should tailor the spec reporter output specific to Node core. I think a better way forward is #52189. We can use a custom reporter instead of the spec one and make it look exactly how we want for core.

[legendecas]

This issue is generic about the spec reporter. I complain about it is because I use it in node core development most often. But I don't think this is specific to node core. Agree that we can create a dedicated reporter for node core and this is still a worth change.

Which one will you keep?

Maybe we can keep the last one, it could look like this:

(assuming lots of passing tests...)
...
✖ foo (0.308375ms)
...
(assuming lots of passing tests...)

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 50.714375

✖ failing tests:

test at t.js:3:1
✖ foo (0.308375ms)
  Error: foo
      at TestContext.<anonymous> (/Users/cwu631/Developer/nodejs/node/test.js:4:9)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:931:25)
      at Test.start (node:internal/test_runner/test:829:17)
      at startSubtestAfterBootstrap (node:internal/test_runner/harness:297:17