test_runner: refactor coverage report output for readability by dmnsgn · Pull Request #47791 · nodejs/node

dmnsgn · 2023-04-30T13:05:32Z

Add a "table" parameter to getCoverageReport.
Keep the tap coverage output intact.
Change the output by adding padding and truncating the tables' cells. Add separation lines for table head/body/foot.
Group uncovered lines as ranges.
Add yellow color for coverage between 50 and 90.

Refs: #46674

nodejs-github-bot · 2023-04-30T13:05:36Z

Review requested:

@nodejs/test_runner

MoLow · 2023-04-30T13:21:52Z

-  }
-  return coverage;
+function addTableLine(prefix, width) {
+  return `${prefix}${'-'.repeat(width)}\n`;


Suggested change

return `${prefix}${'-'.repeat(width)}\n`;

return `${prefix}${StringPrototypeRepeat('-', width)}\n`;

also consider memoizing, like https://github.com/nodejs/node/blob/102540aef9024ea6a8218a6d782243f1391612d2/lib/internal/test_runner/reporter/tap.js#L88-97

MoLow · 2023-04-30T13:38:30Z

Thanks for this PR, it really improves the output of the coverage report.

Refs: nodejs#47791

dmnsgn · 2023-04-30T19:25:27Z

@MoLow Thanks! I have now implemented all your feedbacks.

cjihrig · 2023-05-24T21:32:17Z

@dmnsgn are you planning to continue working on this?

dmnsgn · 2023-05-25T09:41:57Z

@dmnsgn are you planning to continue working on this?

All yours and @MoLow comments have been implemented and I think that's as much I can contribute as of now.
Hope this helps and happy to leave it in your hands from now on.

dmnsgn · 2023-06-05T22:46:13Z

Conflicts all fixed as well.

MoLow · 2023-06-06T04:22:37Z

can you please squash this branch / omit the merge commit?

Add a "table" parameter to getCoverageReport. Keep the tap coverage output intact. Change the output by adding padding and truncating the tables' cells. Add separation lines for table head/body/foot. Group uncovered lines as ranges. Add yellow color for coverage between 50 and 90. Refs: nodejs#46674

Refs: nodejs#47791

…stant

… for truncate methods

… report getCell

dmnsgn · 2023-06-06T16:10:14Z

@MoLow done, I rebased it.

nodejs-github-bot · 2023-06-06T21:13:40Z

CI: https://ci.nodejs.org/job/node-test-pull-request/52133/

nodejs-github-bot · 2023-06-07T07:38:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/52139/

nodejs-github-bot · 2023-06-07T09:08:29Z

Landed in 23c7f65

piranna · 2023-04-30T17:25:54Z

+function formatLinesToRanges(values) {
+  return ArrayPrototypeMap(ArrayPrototypeReduce(values, (prev, current, index, array) => {
+    if ((index > 0) && ((current - array[index - 1]) === 1)) {
+      prev[prev.length - 1][1] = current;


Use .at(-1)?

piranna · 2023-04-30T17:27:52Z

+    if (truncate) result = truncate(result, width);
+    if (color && coverage !== undefined) {
+      if (coverage > 90) return `${coverageColors.high}${result}${color}`;
+      if (coverage > 50) return `${coverageColors.medium}${result}${color}`;


Not sure if these values should be configurable someway...

piranna · 2023-06-07T09:15:31Z

I missed to publish my review before, anyway good to know this has landed :-)

Add a "table" parameter to getCoverageReport. Keep the tap coverage output intact. Change the output by adding padding and truncating the tables' cells. Add separation lines for table head/body/foot. Group uncovered lines as ranges. Add yellow color for coverage between 50 and 90. Refs: #46674 PR-URL: #47791 Reviewed-By: Moshe Atlow <[email protected]>

Add a "table" parameter to getCoverageReport. Keep the tap coverage output intact. Change the output by adding padding and truncating the tables' cells. Add separation lines for table head/body/foot. Group uncovered lines as ranges. Add yellow color for coverage between 50 and 90. Refs: nodejs#46674 PR-URL: nodejs#47791 Reviewed-By: Moshe Atlow <[email protected]>

Add a "table" parameter to getCoverageReport. Keep the tap coverage output intact. Change the output by adding padding and truncating the tables' cells. Add separation lines for table head/body/foot. Group uncovered lines as ranges. Add yellow color for coverage between 50 and 90. Refs: #46674 PR-URL: #47791 Reviewed-By: Moshe Atlow <[email protected]>

nodejs-github-bot added dont-land-on-v14.x needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Apr 30, 2023

dmnsgn mentioned this pull request Apr 30, 2023

test_runner: add code coverage support to spec reporter #46674

Merged

MoLow reviewed Apr 30, 2023

View reviewed changes

Comment thread lib/internal/test_runner/utils.js Outdated

MoLow reviewed Apr 30, 2023

View reviewed changes

Comment thread lib/internal/test_runner/utils.js Outdated

MoLow reviewed Apr 30, 2023

View reviewed changes

Comment thread lib/internal/test_runner/utils.js Outdated

MoLow reviewed Apr 30, 2023

View reviewed changes

Comment thread lib/internal/test_runner/utils.js Outdated

MoLow reviewed Apr 30, 2023

View reviewed changes

Comment thread lib/internal/test_runner/utils.js Outdated

dmnsgn added a commit to dmnsgn/node that referenced this pull request Apr 30, 2023

test_runner: use yellow from util/colors

fb57848

Refs: nodejs#47791

ljharb reviewed Apr 30, 2023

View reviewed changes

Comment thread lib/internal/test_runner/utils.js Outdated

targos removed the dont-land-on-v14.x label May 3, 2023

dmnsgn added 10 commits June 6, 2023 17:03

lib: add yellow to colors util

be8cdf8

test_runner: use yellow from util/colors

e024399

Refs: nodejs#47791

test_runner: memoize coverage report table line

6bbe5b7

test_runner: use primordial StringPrototypeRepeat

1f9ad76

test_runner: extract horizontal ellipsis unicode character into a con…

d22b485

…stant

test_runner: use StringPrototypeSlice primordial instead of substring…

04d2b78

… for truncate methods

test_runner: default coverage report availableWidth to Infinity

b7cdf96

test_runner: pass pad, truncate and coverage as arguments to coverage…

dbafa5b

… report getCell

test_runner: remove quotes for coverageColors properties

98a327b

dmnsgn force-pushed the coverage-spec-table-format branch from c86191b to 98a327b Compare June 6, 2023 16:06

MoLow reviewed Jun 6, 2023

View reviewed changes

Comment thread lib/internal/test_runner/utils.js Outdated

Update lib/internal/test_runner/utils.js

fdade11

MoLow approved these changes Jun 6, 2023

View reviewed changes

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 6, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 6, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 7, 2023

nodejs-github-bot merged commit 23c7f65 into nodejs:main Jun 7, 2023

piranna reviewed Jun 7, 2023

View reviewed changes

RafaelGSS mentioned this pull request Jul 3, 2023

v20.4.0 proposal #48643

Merged

ruyadorno mentioned this pull request Aug 29, 2023

v18.18.0 proposal #49220

Merged

	return `${prefix}${'-'.repeat(width)}\n`;
	return `${prefix}${StringPrototypeRepeat('-', width)}\n`;

Uh oh!

Conversation

dmnsgn commented Apr 30, 2023

Uh oh!

nodejs-github-bot commented Apr 30, 2023

Uh oh!

Uh oh!

MoLow Apr 30, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MoLow commented Apr 30, 2023

Uh oh!

dmnsgn commented Apr 30, 2023

Uh oh!

Uh oh!

cjihrig commented May 24, 2023

Uh oh!

dmnsgn commented May 25, 2023

Uh oh!

dmnsgn commented Jun 5, 2023

Uh oh!

MoLow commented Jun 6, 2023

Uh oh!

dmnsgn commented Jun 6, 2023

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 6, 2023

Uh oh!

nodejs-github-bot commented Jun 7, 2023

Uh oh!

nodejs-github-bot commented Jun 7, 2023

Uh oh!

piranna Apr 30, 2023

Choose a reason for hiding this comment

Uh oh!

piranna Apr 30, 2023

Choose a reason for hiding this comment

Uh oh!

piranna commented Jun 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants