GeezerVison

seems like this is duplicated from the tap reporter, can we extract the code to a common location?

Yes 👍
where we should add this code?
I think internal/util/ or internal/test_runner/utils.js one may be the right location...?

Also, can you please tell me what color and symbol we can use with the spec coverage-reporter?

When I wrote this table for the TAP reporter, I kind of assumed that there is a good chance the TAP will be processed by something else to make it display more nicely. For the spec reporter, I would expect something a little nicer looking out of the box.

maybe coverage should have a separate full-blown reporter?

No strong feeling there.

I don't know how much Node's spec coverage reporter should provide out of the box. But I guess the two common elements for these types of coverage reports would be:

• output statistics formatted into a table
• percentage thresholds indicated by color (green, yellow & red).

At #46665 (comment) and #47032, I suggested since spec reporter is intended to provide an user readable output, I propose to use Istambul text reporter, that's the same being used by Jest, NYC, and by extension also c8.

@piranna how about this:

Looks nice, I like it :-) Just remove the extra whitespace in line % and funcs % columns to save horizontal space.

Are going filenames and number to have some extra colors too, like yellow and red?

Here are some screenshot at different available terminal width:

• Full width

• Medium:

• Small:

With uncovered lines as ranges and filename with colors.
I will prepare a PR if that's okay.

Here are some screenshot at different available terminal width:

I love it!!! 😍

With uncovered lines as ranges and filename with colors.

I would also set yellow for percentages between red and green, and I would remove the comma between the ranges, the whitespace is enough separator. Also in ranges, I would set red for uncovered lines and yellow for uncovered branches, if possible at the same time, then this would be a full replacement for IstanbulJs text reporter :-)

@dmnsgn I think it looks fantastic.

I will prepare a PR if that's okay.

Please do.

PR: #47791 (maintainers edit is set so feel free to take over and do any change you deem necessary)

It seems like we're not validating the important parts here. You can see how I tested this here.

I think we should use a simpler test file than test_runner_output.js for this.

there seems this is still a code duplication

There is a lot of overlap between this file and test-runner-coverage.js. I would add the two tests from this file to the existing file. You could even leverage subtests:

test('spec reporter', async (t) => {
  await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => {});
  // etc.
});

If you do that, please create a similar top level test for the TAP reporter too.

Please remove the default values for symbol and color. The TAP reporter can pass them explicitly. Also, instead of using white for the TAP reporter, please use the empty string (the value of white if colors are not supported).

The first two subtests (--experimental-test-coverage and --test cannot be combined and handles the inspector not being available) are not specific to the TAP reporter and can stay as top level tests.

Should white be used here? I'm trying to avoid generating colors for the TAP reporter. If this is to reset the output color, then maybe do something like:

if (color) {
  report += white
}

Not something that needs to be done in this PR, but we should probably specify --test-reporter tap explicitly for these tests now that the default is sometimes TAP and sometimes spec.

we can do in this pr 👍🏼

Not something that needs to be done in this PR, but we should probably specify --test-reporter tap explicitly for these tests now that the default is sometimes TAP and sometimes spec.

Since tap is more machine friendly and spec more user friendly, I think the latest one should be used by default always, and being tap and opt-in, similar to json output.