Closed dokato closed 3 years ago
Also @jefferis I need a bit more info on what travisbuildchildren.sh
supposed to do?
:exclamation: No coverage uploaded for pull request base (
master@a155984
). Click here to learn what that means. The diff coverage isn/a
.:exclamation: Current head 6ae8c70 differs from pull request most recent head 0bd9463. Consider uploading reports for the commit 0bd9463 to get more accurate results
@@ Coverage Diff @@
## master #475 +/- ##
=========================================
Coverage ? 76.89%
=========================================
Files ? 47
Lines ? 5856
Branches ? 0
=========================================
Hits ? 4503
Misses ? 1353
Partials ? 0
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 a155984...0bd9463. Read the comment docs.
Thanks so much @dokato. I would like to merge quickly, but just wanted to check re coverage. I have been using coveralls for status (there is a badge) but not as a CI check. Were you suggesting a codecov CI check? This might be good.
Also could you summarise the windows issues? I did not have a Windows build in the past, but it might be nice to add one.
Were you suggesting a codecov CI check? This might be good.
Ah, force of habit. I used in previous projects codecov mainly. I can change to coveralls, no problem.
With windows the error comes from installing CMTK. The excerpt from logs is below.
CMake Error at Utilities/numdiff-5.2.1/CMakeLists.txt:155 (MESSAGE):
Declaration of vsprintf() not present!
You can see the whole logs here.
I wasn't sure if there's an easy way to install from exe
using windows command line.
BTW I checked also curls from travisbuildchildren.sh
, they seem not to work now. Do we need some replacement here too?
@jefferis I had a first go with coveralls, looks like we need to have COVERALLS_TOKEN
set up: https://github.com/r-lib/covr#coveralls. It probably wasn't needed for travis.
Now it failed:
Error in covr::coveralls() :
Failed to upload coverage data. Reply by Coveralls: Couldn't find a repository matching this job.
@dokato, I added the COVERALLS_TOKEN as a secret and reran the jobs, but that still failed. I forgot, but I think it needs to be exposed explicitly in the workflow doc.
This is really frustrating re coveralls. The best lead I have found is here. But I think we should already be fulfilling everything that he says.
Thanks @jefferis, actually just before lunch I found this thread with your comments from April ;) I'm testing 2 more things from this user advice. If that doesn't work, are you happy to migrate to codecov?
If that doesn't work, are you happy to migrate to codecov?
Sure. I do have a lot of coverage history but the end of the day we're mostly interested in current state and the next delta.
Before I do that, are you sure @jefferis that the coveralls repo token is correctly set up as secret? I tested on my repo (todor
) locally and with GA and it worked just fine: https://github.com/dokato/todor/runs/3036482539?check_suite_focus=true#step:12:14
I.e.
> Rscript -e 'covr::coveralls(service_name="jenkins")'
$message
[1] "Job ##3.1"
$url
[1] "https://coveralls.io/jobs/83379076"
Could you share with me this token through some vault? I don't have privilidges for nat to see it here.
Thanks for all of this @dokato. As you can see I have just gone with your codecov branch – not worth going against the current for something not mission critical!
Related to #450.
I had a quick try on github actions, I can't quite figure out why Windows CMTK installation fails... I also temporarily switched of ubuntu release as it was taking forever to pull down dependencies. This should work no problem though. If we want to get unblock for now, I suggest testing on mac/ubuntu now and figure the rest later?