build
build copied to clipboard
nodejs
Redirect coverage.nodejs.org to codecov.io
Are we happy enough with codecove.io's coverage tracking that we could consider redirecting coverage.nodejs.org to its results?
The advantage being that our nightly coverage jobs have had a habit of breaking throughout the years.
CC: @mhdawson, @Trott
I'll defer to @Trott on this one since he probably looks at the coverage output on a more regular basis. If he's comfortable then I'd be ok as well.
Having said that I'm trying to look at some of the results and getting this:
CC: @thomasrockhu we still seem to be having some oddities with displaying reports occasionally:
Seems to work for me:
Yes, I looked at one ok, but then when I selected another I got the error I showed. After that I could not find one that would display.
Guy's what this coverage is all about? Code which we wrote and use right? Or it is related to testing. Please tell I'm interested to work on this.
@mhdawson, @bcoe looks fine on my end as well (logged in and out). If you come across another of if this is still an issue, let me know!
I still get this from 2 different machines. I even tried using firefox and still get the same thing:
https://codecov.io/gh/nodejs/node/src/master/lib/_http_incoming.js
@thomasrockhu FYI, seems to be an ongoing issue, not sure why its affects some but not all.
Thanks @mhdawson, looks like there is a bug on our end. Our current suspicion is that users that are not logged in cannot access the page. This is NOT the intended behavior, and we will work on a fix.
Please note that it is the holiday season, so it may take some time to roll out the patch as a significant portion of our team is off.
Guy's what this coverage is all about? Code which we wrote and use right? Or it is related to testing. Please tell I'm interested to work on this.
Hey @slimcoders, we track the coverage of the lines of code that unit tests in the Node.js codebase touch. These reports can represent a good starting point for people to make contributions to Node.js, by writing unit tests for parts of the codebase that are missing coverage, see:
https://coverage.nodejs.org/
For reports.
I observe that there are so many lines not covered. so i can work on that?
@slimcoders that is one of the reasons we generate/publish the coverage info. Many PRs to increase coverage have come from that so it is a good way to contribute :)
One way to a quick resolution might be a landing page that links to both. It will be an extra click, but I think that would be OK. If we could do anonymized click-tracking on the page to see which link people use more often, even better.
I'd want the following questions answered if we were to switch directly to codecov.io at this time:
- How would people get to the existing coverage.nodejs.org? Would it be completely inaccessible?
- The C++ coverage difference between the two is very large. What's the explanation for that?
- ~In the existing coverage, I almost exclusively look at and sort by branch coverage instead of line coverage. I can try to switch my workflow, but I'd rather not if I don't have to. I think branch coverage is the most useful metric. Any chance at all that will be implemented in codecov.io? Is it an option I need to enable or something?~
I just tried looking at codecov.io stats in detail for the first time in a while and it looks like maybe our config needs a tweak? Clicking through to JavaScript files doesn't show the source but shows an error message instead. When I end up at https://codecov.io/gh/nodejs/node/src/master/lib/async_hooks.js, I get a GitHub API Forbidden error:
@Trott see https://github.com/nodejs/nodejs.org/issues/3583#issuecomment-749137557.
Seems like my branches question can be ignored. The way the line coverage vs. partial line coverage stuff is displayed in codecov.io seems to surface the same information in a different way. I can use that.
IIRC the issue should be part of the build repo since that's where the redirects happen.
So what's the consensus here -- are we ready to replace coverage.nodejs.org with codecov.io? As one of only two people keeping coverage.nodejs.org working I'd be in favouring of switching over.
So what's the consensus here -- are we ready to replace coverage.nodejs.org with codecov.io? As one of only two people keeping coverage.nodejs.org working I'd be in favouring of switching over.
I'm on board with whatever minimizes the maintenance burden of providing this information.
I'd really like us to switch over as well if it provides the information needed.
@Trott do you look at it/use it regularly? Every time we discuss I do a sniff test and don't seem to be able to see the data. Today when I try to get coverage info for one of the C files I just get an error.
If it's regularly used by others and I'm just unlucky I'm still happy to switch over.
If I try to open the coverage for node_buffer.cc, I see no error, but the page reports 0% of coverage, which is weird because in the files list, it has something:
It seems to work fine for JS files, so maybe Codecov doesn't support C++ ?
@targos, I'll dig into why you aren't seeing coverage on that file. We support C++ and this is likely a bug in our new file viewer.
I'm not signed in to codecov. To sign in it seems to want a bunch of organization access.
@mhdawson you shouldn't need to sign in, but just wanted to see if it was a different error. I'll try to repro this today and get a fix.
@mhdawson, this should be fixed now. @targos, we also pushed a fix, you should see coverage on the fileviewer.
@thomasrockhu thanks, seems to be working now.
I'll still defer to those who use coverage regularly to chime in on whether we should switch over.
I'll still defer to those who use coverage regularly
That would be me. I wrote:
I'm on board with whatever minimizes the maintenance burden of providing this information.
Which brings us to what @richardlau wrote:
As one of only two people keeping coverage.nodejs.org working I'd be in favouring of switching over.
So I think we're good to switch.
Not directly relevant to the "should we switch or not" question, but if there's a way to edit the home/dashboard page, that would be helpful, I think. Specifically, the coverage chart, coverage sunburst, and recent commits list are not terribly useful for us. If they could be removed or at least moved below the directory/coverage listing at the bottom, that would be great.
Here's what I'm talking about by "directory/coverage listing". It's the super useful thing that we want people to see first:
@Trott currently this isn't possible, but we are working on a UI redesign of this page. I'll let our product team know.