test_runner: refactor coverage report output for readability #47791
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review requested:
|
nodejs-github-bot
added
dont-land-on-v14.x
needs-ci
MoLow
reviewed
Apr 30, 2023
MoLow
reviewed
Apr 30, 2023
lib/internal/test_runner/utils.js
Outdated
| } | ||
| return coverage; | ||
| function addTableLine(prefix, width) { | ||
| return `${prefix}${'-'.repeat(width)}\n`; |
There was a problem hiding this comment.
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
reviewed
Apr 30, 2023
MoLow
reviewed
Apr 30, 2023
MoLow
reviewed
Apr 30, 2023
MoLow
reviewed
Apr 30, 2023
|
Thanks for this PR, it really improves the output of the coverage report. |
dmnsgn
added a commit
to dmnsgn/node
that referenced
this pull request
Apr 30, 2023
|
@MoLow Thanks! I have now implemented all your feedbacks. |
ljharb
reviewed
Apr 30, 2023
|
@dmnsgn are you planning to continue working on this? |
|
Conflicts all fixed as well. |
|
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
… for truncate methods
dmnsgn
force-pushed
the
coverage-spec-table-format
branch
from
c86191b to
98a327b
Compare
|
@MoLow done, I rebased it. |
MoLow
reviewed
Jun 6, 2023
MoLow
approved these changes
Jun 6, 2023
github-actions
bot
removed
the
request-ci
MoLow
added
author ready
nodejs-github-bot
removed
the
commit-queue
|
Landed in 23c7f65 |
piranna
reviewed
Jun 7, 2023
| 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; |
| 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}`; |
There was a problem hiding this comment.
Not sure if these values should be configurable someway...
|
I missed to publish my review before, anyway good to know this has landed :-) |
This was referenced Jun 9, 2023
RafaelGSS
pushed a commit
that referenced
this pull request
Jul 3, 2023
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 <moshe@atlow.co.il>
Merged
Ceres6
pushed a commit
to Ceres6/node
that referenced
this pull request
Aug 14, 2023
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 <moshe@atlow.co.il>
Ceres6
pushed a commit
to Ceres6/node
that referenced
this pull request
Aug 14, 2023
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 <moshe@atlow.co.il>
ruyadorno
pushed a commit
that referenced
this pull request
Aug 29, 2023
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 <moshe@atlow.co.il>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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