Repository navigation
Only print failing assertion once with spec reporter #56316
Description
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.
Activity
pmarchini commentedon Dec 19, 2024
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 commentedon Dec 19, 2024
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.
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 commentedon Dec 19, 2024
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.
Maybe we can keep the last one, it could look like this:
Llorx commentedon Dec 21, 2024
I'm also with the last one. I will pickup this issue this incoming week as I'm on PTO.