test: add coverage GitHub action

[bcoe]

This is a first attempt at adding a GitHub Action that pushes coverage results to codecov.

Fixes #35646

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

CC: @nodejs/testing

[watilde]

It's almost working, but the below three errors on CI seem to be a cause of the workflow failure:

file: internal/error-serdes.js error: Error: ENOENT: no such file or directory, open '/home/runner/work/node/node/lib/internal/error-serdes.js'
file: internal/process/main_thread_only.js error: Error: ENOENT: no such file or directory, open '/home/runner/work/node/node/lib/internal/process/main_thread_only.js'
file: internal/process/stdio.js error: Error: ENOENT: no such file or directory, open '/home/runner/work/node/node/lib/internal/process/stdio.js'

[codecov-io]

Codecov Report

Merging #35653 into master will decrease coverage by 0.47%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #35653      +/-   ##
==========================================
- Coverage   96.87%   96.40%   -0.48%     
==========================================
  Files         212      220       +8     
  Lines       69641    73672    +4031     
==========================================
+ Hits        67466    71024    +3558     
- Misses       2175     2648     +473     

Impacted Files	Coverage Δ
lib/internal/crypto/scrypt.js	28.33% <0.00%> (-71.67%)	⬇️
lib/internal/crypto/diffiehellman.js	92.47% <0.00%> (-6.50%)	⬇️
lib/internal/crypto/keys.js	94.99% <0.00%> (-4.22%)	⬇️
lib/internal/crypto/util.js	96.79% <0.00%> (-3.21%)	⬇️
lib/internal/crypto/sig.js	97.54% <0.00%> (-2.46%)	⬇️
lib/internal/source_map/prepare_stack_trace.js	94.95% <0.00%> (-1.69%)	⬇️
lib/internal/crypto/keygen.js	99.20% <0.00%> (-0.49%)	⬇️
lib/_http_server.js	98.34% <0.00%> (-0.19%)	⬇️
lib/fs.js	94.13% <0.00%> (-0.13%)	⬇️
lib/internal/modules/esm/translators.js	95.14% <0.00%> (-0.12%)	⬇️
... and 49 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2002d90...48de3fc. Read the comment docs.

[targos]

I don't know if it's an issue. Codecov found many extra "reports":

==> Reading reports
    + ./tools/doc/coverage/tmp/coverage-5164-1602748881271-0.json bytes=890994
    + ./.github/workflows/coverage-linux.yml bytes=903
    + ./out/Release/.deps/home/runner/work/node/node/out/Release/obj.target/v8_initializers/gen/deps/v8/src/builtins/internal-coverage-tq-csa.o.d bytes=81699
    + ./out/Release/.deps/home/runner/work/node/node/out/Release/obj.target/v8_base_without_compiler/deps/v8/src/debug/debug-coverage.o.d bytes=31478
    + ./out/Release/obj/gen/deps/v8/src/builtins/internal-coverage-tq-csa.cc bytes=23084
    + ./coverage/lcov.info bytes=1279096
    + ./deps/v8/test/inspector/cpu-profiler/coverage-expected.txt bytes=24142
    + ./deps/v8/test/inspector/cpu-profiler/coverage-block-expected.txt bytes=30502
    + ./deps/v8/test/inspector/debugger/side-effect-free-coverage-enabled-expected.txt bytes=92
    + ./deps/v8/src/builtins/internal-coverage.tq bytes=1480
    + ./deps/v8/src/debug/debug-coverage.cc bytes=28807

[Trott]

Non-blocking question: IIRC, the current nightly job combines Windows and POSIX coverage at codecov. Is the long-term plan to try to get that happening with this too? If so, does it make sense to add a "TODO" or something?

[bcoe]

@thomasrockhu before we land this, I wanted to see if you have any feedback. My main questions being:

1. our coverage report is quite large for the Node.js codebase, is it any concern on your end that we'd be uploading it for each PR?
2. any recommendations on how we approach our configuration, keeping in mind we'd like to eventually combine Windows and Linux reports (so the upload will come in two parts) -- the concrete question I have about this, is how we avoid spamming the PR thread with multiple Codecov Reports, since the uploads will arrive out of sync.

[thomasrockhu]

Hi @bcoe

1. The current upload is only about 5MB, this is below any concerning upload size, so I think we should be good here
2. As for configuration, this looks pretty standard, as I'm guessing there will be other jobs that will be running Windows builds. The only concern I have is if you start running a significant number of matrix jobs (which would result in uploading dozens and dozens of reports). My suggestion there would be to batch coverage reports in one Codecov upload job, but I don't think this is going to be a problem in the near future.
   As far as multiple Codecov reports, we generally post one comment and update it as we ingest coverage reports. This means you should only get one Codecov PR comment per PR.
   If you are concerned about getting a comment prematurely, there is the after_n_builds flag that will prevent notifications until we receive n number of uploads.

[bcoe]

I'm a little flummoxed as to why having NODE_V8_COVERAGE inline works great, whereas having it in the global env: stanza resulted in bad counts ... but there you have it, it's worked twice in a row with the environment variable here.

[bcoe]

we can ratchet this number up as we improve the linux coverage #s, and it will result in a failure to the action if it drops below the threshold.

[bcoe]

If you are concerned about getting a comment prematurely, there is the after_n_builds flag that will prevent notifications until we receive n number of uploads.

@thomasrockhu thank you 👏 ,

Sounds like we'll just turn this setting on if the delayed merging becomes an annoyance.

I'm really excited to get coverage as part of our immediate feedback loop, and appreciate the help you've given.

I don't know if it's an issue. Codecov found many extra "reports"

@targos I've fixed it so we should only be uploading one report from the coverage/ folder.

[nodejs-github-bot]

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

[bcoe]

It's almost working, but the below three errors on CI seem to be a cause of the workflow failure:

@watilde I believe those errors you mentioned are false positives negatives in the c8 reporter, from file paths it had trouble opening (I'm betting they're just warnings).

It would be good to dig into these warnings, and into the three tests that fail under coverage. If we land this, and make coverage one of the signals on PRs, perhaps it will help motivate us to dig into these problems.

[Trott]

Landed in bb2945e

[BethGriggs]

@bcoe is this PR expected to land on the v14.x release line?

I cherry-picked it into the v14.15.2-proposal, but the action is failing on the release proposal PR (refs: #36476 (comment)). Just trying to figure out if there's real failure we need to address, or the PR should not land on v14.x.

[bcoe]

@BethGriggs I think it's okay to keep this action on the main branch, and not back-port. My hope is that it gives us insights into our test coverage going forward.