Node: Coverage report left in comments is "noisy"

[bcoe]

Problem

I noticed on a couple PRs, that the coverage comment left by codecov.io was hidden.

I'm guessing that part of why folks wanted to hide this notification, is that it takes up a lot of screen space on the PR 👇

Proposed Solution

I have a PR open here that:

• enables coverage for a couple more platforms.
• uses codecov's layout setting to reduce the screen real-estate used by a coverage report.
• only posts the report if coverage has changed.

What if folks still aren't happy with the report?

If folks are still finding they find coverage a little bit noisy (and are inclined to hide the comment), we can turn off the comments in configuration.

@nodejs/testing

[aduh95]

If folks are still finding they find coverage a little bit noisy (and are inclined to hide the comment), we can turn off the comments in configuration.

Could we change the comment content to put it in a <detail> tag?

[bcoe]

Could we change the comment content to put it in a tag?

I didn't see a way to do this in the configuration. @thomasrockhu is there a way to customize the coverage report, so that some of the information is hidden under a <detail> rollup?

[gengjiawen]

Maybe also skip draft PR. Pretty annoying in such case.

[richardlau]

We could also skip coverage for doc-only changes like we do with the asan workflow:
https://github.com/nodejs/node/blob/c55f661551d7368778ae6f9adc584467151bce8a/.github/workflows/test-asan.yml#L13-L14

[thomasrockhu]

@bcoe, I don't believe that's possible, but that's really good feedback. I'll bring it to the product team to see what we can do here, but I'm not sure what the turnaround time would be. What's the ideal result for you?

@gengjiawen, also great feedback. Thanks for bringing this up!

[aduh95]

@thomasrockhu Ideally, something like that:

Merging #35670 into master will decrease coverage by 7.59%.

@@            Coverage Diff             @@
##           master   #35670      +/-   ##
==========================================
- Coverage   96.40%   88.80%   -7.60%     
==========================================
  Files         220      474     +254     
  Lines       73681   112964   +39283     
  Branches        0     9079    +9079     
==========================================
+ Hits        71031   100319   +29288     
- Misses       2650     7033    +4383     
- Partials        0     5612    +5612     

| Impacted Files | Coverage Δ | |
|---|---|---|
| src/node_i18n.cc | 76.00% <0.00%> (ø) | |
| src/inspector/worker_inspector.cc | 95.52% <0.00%> (ø) | |
| src/node.h | 93.33% <0.00%> (ø) | |
| src/inspector_agent.h | 87.50% <0.00%> (ø) | |
| src/inspector/node_string.cc | 54.28% <0.00%> (ø) | |
| src/api/hooks.cc | 80.18% <0.00%> (ø) | |
| src/env.cc | 87.19% <0.00%> (ø) | |
| src/node_contextify.h | 70.83% <0.00%> (ø) | |
| src/inspector/worker_agent.cc | 89.61% <0.00%> (ø) | |
| src/node_os.cc | 75.63% <0.00%> (ø) | |
| ... and 264 more | |

---

The most relevant info is still available at a glance, and it doesn't take forever to scroll by.

[thomasrockhu]

@aduh95, awesome, we'll look into making this configurable in the yaml. Thanks again for the feedback!