3384 Unpin coverage

[sajith]

For 3384. Motivation is to try and see if coveralls.io works better for us; see 3385 for relevant ticket.

[codecov-commenter]

Codecov Report

Merging #783 into master will increase coverage by 0%.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #783   +/-   ##
======================================
  Coverage      91%     91%           
======================================
  Files         155     155           
  Lines       27277   27277           
  Branches     3879    3879           
======================================
+ Hits        24936   24951   +15     
+ Misses       1607    1597   -10     
+ Partials      734     729    -5     

Impacted Files	Coverage Δ
src/allmydata/web/status.py	95% <0%> (+<1%)	⬆️
src/allmydata/immutable/checker.py	88% <0%> (+1%)	⬆️
src/allmydata/immutable/downloader/share.py	95% <0%> (+2%)	⬆️

---

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 b79e9c4...50fb793. Read the comment docs.

[exarkun]

It would be good for the ticket to explain why this change is a good idea. I infer it is to support the switch to coveralls.io. It would be good if that ticket would explain why that change is a good idea. :)

[sajith]

Ah yes, certainly! Updated 3384.

[meejah]

LGTM

[exarkun]

Just a couple notes.

Though codecov says "coverage is unaffected" what it really means is "the number of new uncovered lines exactly equals the number of new covered lines". The changes are probably spurious, due to heisencoverage or some such, but it seems worth noting and understanding since this is a PR to change how coverage is measured.

Also, did unpinning coverage actually cause coverage 5.x to be used? Or was the cached coverage from yesterday's Docker image builds used? Also, related, the NixOS job has a different way that it picks Python libraries. I would assume that it is still using 4.x (and so the switch is not yet complete).

[sajith]

I guess the objection to codecov is the amount of weird voodoo: this PR, for example, has only changed one line in setup.py, and added some lines in newsfragments and .gitignore, but per the comment above some source files are impacted. I for one ignore those comments because I often can't make sense of it.

It must be a comparison against something, but it's not clear if the comparison is with test coverage on master branch, even though that is the claim. Perhaps the comparison is against the last uploaded report? Perhaps those numbers are imprecise because they're using floating points? I don't know... I also don't know if coveralls would be an improvement, to be fair to codecov. :-)

On GitHub Actions, we're certainly using coverage 5.x, per the log.

What do you think about using GitHub Actions for uploading coverage reports, and maybe removing the part that uploads coverage to codecov from CircleCI configuration? On GitHub Actions, we could also add ubuntu-latest to coverage test matrix, so that reports for all three platforms are covered. It doesn't add any more time to the time we spend waiting on CI: we'd be waiting for CircleCI to finish running anyway.

[sajith]

On CircleCI also, we're now using coverage==5.2.1, except on Ubuntu 20.04 and NixOS.

I just noticed that Ubuntu 20.04 images are likely stale. Ubuntu 20.04 image generation has been failing for the last eight days. Is that expected?

[exarkun]

I guess the objection to codecov is the amount of weird voodoo:

I'm not sure if this is a response to my comment or not. I've certainly seen weird reports from codecov (my last comment gave an example of such a result). What's not so clear to me is what subset of this weirdness is attributable to codecov itself. The main point I wanted to make is that this PR (a) has a change in coverage results reported by codecov and (b) changes the tool that is used to measure coverage. Is the explanation that codecov is being weird again? Or that the tool is actually recording different coverage now? It seems like a useful question to have answered. One idea would be to run the test suite locally with both versions of coverage and see if it is possible to get the same coverage results or not.

Ubuntu 20.04 image generation has been failing for the last eight days. Is that expected?

No, that's not expected. :(

[exarkun]

On GitHub Actions, we could also add ubuntu-latest to coverage test matrix, so that reports for all three platforms are covered. It doesn't add any more time to the time we spend waiting on CI: we'd be waiting for CircleCI to finish running anyway.

I don't think this is a good solution. One CI service or another for each platform should be sufficient. I doubt the Ubuntu 20.04 image building failures have anything to do with CircleCI in particular, that's just where we happen to be trying to test Ubuntu 20.04. If we want to move to GitHub Actions, fine (not fine 😉) - but let's not duplicate stuff in an ad hoc fashion.

[sajith]

I'm not sure if this is a response to my comment or not.

No, it is not. :-)

Just laying out some complaints about codecov. Take #779, for another example: there's nothing really objectionable about test coverage of that PR, but codecov/patch won't be happy until there's test coverage for error messages. Adding coverage for if PY3: branches that will be eventually dropped anyway seems excessive to me.

One idea would be to run the test suite locally with both versions of coverage and see if it is possible to get the same coverage results or not.

Good idea! I will report some results.

[sajith]

Just laying out some complaints about codecov. Take #779, for another example: there's nothing really objectionable about test coverage of that PR, but codecov/patch won't be happy until there's test coverage for error messages. Adding coverage for if PY3: branches that will be eventually dropped anyway seems excessive to me.

I must also admit that I have found codecov/patch helpful on several occasions. There must be some knobs that we can turn, at least for the duration of Python 3 porting? There are some tunable parameters in codecov.yml, such as coverage.precision and coverage.range, which looks useful.

In the case of #779, "90.90% of diff hit (target 91.48%)" isn't that much...

[sajith]

A possibly ignorable problem I noticed when reporting to codecov:

==> Collecting reports
    + tahoe-lafs/coverage.xml bytes=1213491
    + tahoe-lafs/misc/coding_tools/coverage.el bytes=4634

But coverage.el isn't a coverage report...

[exarkun]

A possibly ignorable problem I noticed when reporting to codecov:

==> Collecting reports
    + tahoe-lafs/coverage.xml bytes=1213491
    + tahoe-lafs/misc/coding_tools/coverage.el bytes=4634

But coverage.el isn't a coverage report...

Ha cool. I wonder what kind of coverage report it thinks .el files are.

[sajith]

I uploaded coverage some coverage data from this branch to codecov.io and coveralls.io:

• With coverage 5.2.1:
  • codecov reports 91.01% absolute coverage.
  • coveralls reports 92% coverage.
• With coverage 4.5.4:
  • codecov reports 91.00% absolute coverage.
  • coveralls can't handle coverage's older JSON-ish reports, or the XML report (or I have not been able to figure out how to submit them): it needs coverage 5.x SQLite reports, or lcov.

[sajith]

When I submitted that locally-generated coverage 4.5.4 report, codecov produced a URL. That URL turned up a blank page, presumably because it encountered a report without a corresponding commit in the repository. So I pushed f558f80.

That seems to have turned codecov/project red on this PR. All that commit has changed is one line in setup.py. This is the weird voodoo I'm talking about!

Maybe some of those knobs could be turned, so that what looks like rounding errors should not produce so much noise.

[sajith]

Trying out some codecov configuration in this branch. If this works as expected, I will submit them as a separate PR.

[exarkun]

Where does this PR currently stand?

[sajith]

Where does this PR currently stand?

Upgrading coverage itself seems to be harmless: codecov is (should be -- with caveats) uploading coverage.xml anyway, so immediate output format of coverage is not very relevant there. Maybe we will get some bugfixes if we upgrade coverage from 4.5 to 5.3.

About switching to coveralls (which was originally the eventual goal of this PR), I am uncertain. Perhaps PR #789 that changes some codecov defaults will work well enough for us. I added those changes to this PR branch first though, so this here is a mess.

I think I will close this PR and start afresh.

[sajith]

But coverage.el isn't a coverage report...

Made a PR to fix this: codecov/codecov-python/pull/297