nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.52k stars 29.03k forks source link

Proposal: always default test runner to use spec reporter #54540

Closed cjihrig closed 2 weeks ago

cjihrig commented 2 weeks ago

What is the problem this feature will solve?

The test runner currently uses the more human friendly spec reporter when a TTY output is detected. Otherwise, it defaults to generating TAP output. I think the cons of this behavior outweigh the pros.

In Node core, this would remove occurrences of nested TAP output which can cause issues like https://github.com/nodejs/node/issues/54535. We also have a number of places throughout the codebase, such as the GitHub Actions config, where we force the spec reporter anyway.

In userland, it is a DX improvement (no more TAP output in CI runs). TAP output would still be available via the --test-reporter=tap

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

Make the default test runner reporter be spec.

Notes:

What alternatives have you considered?

No response

targos commented 2 weeks ago

I'm +1, but I've never had a TAP parser in my pipeline so I don't really know if the impact is big.

cjihrig commented 2 weeks ago

The impact would be for people who want TAP output to pass --test-reporter=tap when starting Node. I think that is pretty reasonable for a semver major change.

EDIT: It's worth noting that while this changes a default, passing --test-reporter=tap would be backward compatible to all versions of Node since test reporters were introduced.

RedYetiDev commented 2 weeks ago

Fixed by https://github.com/nodejs/node/pull/54548