updates for v1.5, etc. #289
Conversation
Codecov Report
@@ Coverage Diff @@
## master #289 +/- ##
===========================================
+ Coverage 67.28% 86.70% +19.41%
===========================================
Files 3 3
Lines 162 188 +26
===========================================
+ Hits 109 163 +54
+ Misses 53 25 -28
Continue to review full report at Codecov.
|
|
FWIW, since Travis is ending its unlimited free tier, it might be worth moving this package's CI to GitHub Actions at some point. |
vtjnash
force-pushed
the
jn/updates-v1.5
branch
from
be8a840 to
a99f8f9
Compare
|
Travis was passing earlier, now it seems to be failing. |
vtjnash
force-pushed
the
jn/updates-v1.5
branch
6 times, most recently
from
b72e084 to
f98cd7a
Compare
Coveralls now needs the job id token to be unique, so instead post to a mock server.
vtjnash
force-pushed
the
jn/updates-v1.5
branch
from
f98cd7a to
b1b923d
Compare
Pull Request Test Coverage Report for Build 926
💛 - Coveralls |
And for travis-ci-com, now just seems to confuse Coveralls on our repo.
Keyword args can require a high compile cost, which is not necessary here. Convert to a Dict to avoid that.
I'm assuming their choices of variables are more canonical. For Codecov, I checked the variables against their canonical tool (https://codecov.io/bash). This also adds the ability to specify the Coveralls settings entirely from environment variables, overriding autodetection. There are couple new flags upstream (flags, name, flag_name), and so I've added those here also.
These functions and their tests now live in CoverageTools.jl
vtjnash
force-pushed
the
jn/updates-v1.5
branch
from
a6864d1 to
5169eeb
Compare
|
Yeah, I was still testing different settings and did that intentionally. This has many new updates now, and should be good to review. |
There was a problem hiding this comment.
This PR modifies a lot of things at once. I tried to look at all the changes, and they appear OK, but I won't claim to have checked all of them in detail. They seem broadly sane, though, and if there are regressions, we'll just have to deal with them as they are reported. In the meantime, it seems to unbreak the CI tests and hopefully things in general, so it should be merged.
Thanks!
| global: | ||
| - COVERALLS_PARALLEL=true | ||
| notifications: | ||
| webhooks: https://coveralls.io/webhook |
There was a problem hiding this comment.
I am surprised this is not necessary anymore. They still mention it on https://github.com/marketplace/actions/coveralls-github-action and https://docs.coveralls.io/parallel-build-webhook, and it was absolutely vital when I last played with it -- but that was some time ago; so if that's what you found empirically, let's do it... Coveralls documentation sucks in general, so I am not completely surprised it might not reflect reality anymore :-).
I'll soon need to dig into this again for another project (where Travis cut our legs, so we moved to GitHub Actions, and that broke Coveralls integration), I'll keep this in mind, might simplify things there!
There was a problem hiding this comment.
Yeah, I noticed that even old builds now are showing up correctly without this flag. But the only effect it was now having here was to prevent any builds from showing up (it was unable to process the webhook now, I'm guessing perhaps due to the deprecation of travis-ci.org)
There was a problem hiding this comment.
It used to be disabled entirely, but I'm not sure why it'd be 4 and not 8 (the number of builds) or 1 (the top comment has 5 rolling edits as the parallel builds finished)
There was a problem hiding this comment.
Top comment was created, then edited 5 times; plus 3 more comments; means a total of 1+5+3=9 results reported. The Travis build has 9 jobs. So what I think happens is that whenever one of the jobs ends, Coveralls adds or updates an issue comment. Why it didn't just put all updates in one comment, I can only guess at; most likely a race condition, but w/o being able to see the precise timestamps when each comment was made/updated, it's hard to say...
Fixes #273
Fixes JuliaLang/julia#38173