You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
1. new file paths are breaking the coverage reporting
File paths for Node.js' own files now have this form node:/internal/cluster/shared_handle:
there's now anode: prefix.
there's no longer a .js prefix.
I will happily fix this problem in the test reporter, but could someone point me towards where these changes were made (@watilde, @guybedford?) -- I want to make sure I link to the change in the reporter codebase.
2. nightly coverage is broken again
Coverage has been broken for several nights in a row. I believe this is due to an unrelated CI/CD upgrade?
Suggested fix
let's update c8 to support the new path format (I've tested that this fix is pretty trivial).
let's move code coverage to GitHub Actions (It seems like we need to fix Jenkins once every few months, and I can't see a reason why not to use actions).
@joyeecheung@MylesBorins not a bad idea, but it wouldn't have actually caught this, because it only breaks on Node.js' own coverage -- which now has different file paths (c8 would continue working for userland.)
Activity
aduh95 commentedon Oct 14, 2020
I believe this has been introduced by #35498.
bcoe commentedon Oct 14, 2020
@aduh95 thank you! (sorry for looping in the wrong folks, I assumed it was potentially the new URL parser).
[-]Fixe Node.js' code coverage/code coverage job[/-][+]Fix Node.js' code coverage/code coverage job[/+]joyeecheung commentedon Oct 14, 2020
Have we ever tried adding c8 to https://github.com/nodejs/citgm/ ? (I searched for it and I don't think so?)
MylesBorins commentedon Oct 14, 2020
we should add it, it isn't included right now afaik. For reference this is the lookup table
https://github.com/nodejs/citgm/blob/main/lib/lookup.json
bcoe commentedon Oct 14, 2020
@joyeecheung @MylesBorins not a bad idea, but it wouldn't have actually caught this, because it only breaks on Node.js' own coverage -- which now has different file paths (c8 would continue working for userland.)
test: add windows and C++ coverage
test: add windows and C++ coverage
test: add windows and C++ coverage