Unexpected Coveralls failures on github UI

[jku]

Github shows the red checkmark in many places because of some Coveralls issue:

• Clicking on the supposed failures details link leads me to a Coveralls page https://coveralls.io/jobs/75257608 which does not look like a failure
• I don't understand how the checkmark appears in the list in the first place -- is this some coveralls magic?

[jku]

This seems to have started happening between merges 8101c9e (no additional checks listed) and 768f9d8 (lots of coveralls checks listed) ? Either coveralls backend or the github UI integration must have changed.

I'm still completely baffled by how those Coveralls checks even appear on github?

[lukpueh]

I'm still completely baffled by how those Coveralls checks even appear on github?

Did you see my #1284 (comment)?

I'm pretty sure coveralls.io sends them because of ...

What I don't understand is why it does not send them reliably. I wonder if there's a log somewhere on GitHub or coveralls.io.

[jku]

Thanks, I missed that. You're correct. Confusingly the list in my screenshot includes two different things: checks (which are done in github apps) and statuses (which can be done via API). Example statuses for a commit: https://api.github.com/repos/theupdateframework/tuf/commits/cf2e8466fac17f186323338ad08ab47b63f3f63f/status

The related issues are:

• status state can be "failure" for a job that on coveralls.io is listed as "coverage remained the same" (and the commit literally modified two existing lines)
• only 2 out of 16 of the related jobs were added as github status

These just seem like coveralls bugs to me (but feel free to experiment with the coveralls options). Also there really shouldn't be a separate status for each parallel job but that seems to be their design decision...

[jku]

So based on helpful debugging by @afinetooth what we're seeing is

• a bug in coveralls (it thinks the coverage changed by -0.0% when there was no change)
• this is made worse by our threshold settings that mean any negative change is considered "failure"
• unexplained behaviour where sometimes only some builds appear as github statuses

Additionally:

• the coverage statuses do not seem to appear in the PR where they would be useful! The only appear in for merge commits! This means PRs look just fine but our commit log is full of red ❌ on every merge... Is this really how coveralls is supposed to work?
• I'm not sure if we really want more than a dozen checkmarks for coverage: maybe we just want one (or in the case of PRs, maybe we want a comment instead?)

Potential fixes (these are not dependent on each other):

• set Coverage decrease threshold for failure in coveralls. E.g. a value of 0.01% would currently allow a 3 lines coverage drop per PR I think. Value of 0.001% would allow no coverage drops but probably would remove the false negatives
• maybe remove COVERALLS_FLAG_NAME from our github workflow: this is very badly documented so I don't know what this really does but apparently it leads to us getting a single result in github
• maybe enable github comment (if there is only one result): currently coveralls is IMO pretty useless since it does not report the results in PRs at all... maybe this would 🤷 ?

[lukpueh]

Thanks for investigating this and sharing your insights, @jku! Here's what I think:

• IMO coveralls.io is useful for two things:

  • Notifying a PR author/reviewer that coverage decreased, so that they can add/request more tests (<- does not work)
  • Providing a nice web UI, to browse the code and easily see missed code branches per build, as they might differ between Python versions and platforms (<- works)

• What I think is not useful are:

  • notifications after the fact, i.e. red crosses in the commit log ... we want the PR author to add tests before the merge
  • thresholds that allow coveralls.io to "fail a PR" ... we can and do configure this when invoking the unittest suite, and even there a hard-fail is disputable because it can easily by bypassed (see liberal use of "# pragma: no cover")

Regarding notifications, we did have "PR comment"-type notifications at some point, see e.g. #525 (comment). But IIRC they were also not always reliable. I am unsure now, if we even had "PR status"-type notifications for tuf, as we did in in-toto prior to our switch to GitHub Actions. IIRC those worked quite well, and we only got a single (aggregate?) status notification, despite posting multiple results per build to coveralls.io (one for each tox build). See e.g in-toto/in-toto#425:

So my preferred solution would be:

• continue displaying coverage results per build on coverealls.io, which means keeping COVERALLS_FLAG_NAME
• enable a single aggregate status notification on the PR that does not hard-fail it (one should be enough to make the PR author/reviewer browse the individual results on coveralls.io and add/request more tests)
• disable hard-fail thresholds on coveralls.io altogether

[jku]

this "fixed" itself when the coveralls project recreated itself when we changed github project name (now coveralls checks aren't added: instead we get a PR comment)