Codecov stuff

[jakkdl]

I've been bothered by codecov's github annotations for a long while, where it fairly regularly kept complaining about previously-uncovered code still being uncovered after later commits fixing it. The latest instance of this happened over in pytest but I know it's confused me and others at various points in different trio repositories. (happened in #2812)

When I started digging I found out this functionality is deprecated and has a big red box in the documentation https://docs.codecov.com/docs/github-checks, and I'm fairly certain that more time has been lost collectively from false alarms (and code being near-unreadable in the review window when it's very low on coverage) than from the marginal gain of seeing it in the code review window rather than elsewhere. We'll still have the CI check and everything, and you can easily click through to codecov.io in several places.

I've opened an issue upstream codecov/codecov-action#1710, but in the meantime I think we should edit the YAML at https://app.codecov.io/account/github/python-trio/yaml/

It's a one-line fix, that doesn't even need a PR, but thought I should open it to discussion and make people aware of the change so they don't wonder where it went. And if you personally want the functionality there's a browser extension: https://docs.codecov.com/docs/the-codecov-browser-extension

[webknjaz]

@jakkdl in my experience, one thing that causes Codecov to post statuses prematurely (not just annotations but Checks, comments and their web UI) is allowing it to guess the moment when it's okay to start processing the report files and combining them into one.
I think this is because when there's a matrix of jobs not covering the same lines each, there would be portions of lines for which coverage might end up being reported much later than for many other jobs. And if Codecov decides to process the reports earlier than that, it usually reports lower coverage than it really is. It then updates these statuses later, when that slow job coverage arrives, though. The problem is that the annotations are immutable IIRC and so they can't update them, while the comment and the checks would be.

To fight this, I like setting the config as follows, setting the number of expected uploads explicitly: https://github.com/aio-libs/yarl/blob/f304dd7/.codecov.yml#L3-L8.

[jakkdl]

To fight this, I like setting the config as follows, setting the number of expected uploads explicitly: https://github.com/aio-libs/yarl/blob/f304dd7/.codecov.yml#L3-L8.

ooh, that'd resolve #2689 that has been bugging me forever. Thanks!

But I don't think it'd resolve the problems in the pytest PR where the uncovered code was introduced in one commit, and the tests adding coverage added in a later commit.

The problem is that the annotations are immutable IIRC and so they can't update them, while the comment and the checks would be.

yeah the codecov docs say

The GitHub API does not provide a way to clear annotations once they are sent. Even if subsequent reports processed by Codecov include updated coverage data, previous annotations remain.

[webknjaz]

Yep, that assessment is correct.

[jakkdl]

I've now edited the org-level yaml, could leave this issue open for a bit and wait for impact.

Opened #3156 for the other repo-level improvements

[jakkdl]

With #3156 we're gonna start noticing codecov flakiness 🙃

[2024-12-18T12:31:16.587Z] ['error'] There was an error running the uploader: Error uploading to https://codecov.io: Error: There was an error fetching the storage URL during POST: 503 - upstream connect error or disconnect/reset before headers. reset reason: connection termination

https://github.com/python-trio/trio/actions/runs/12392918438/job/34593105479?pr=3145#step:5:41

[jakkdl]

@webknjaz re: number of uploads - it's not [just] cron, but merge queue + push.
https://app.codecov.io/gh/python-trio/trio/commit/4f66fabf5ca4c2dd40e4e9b76bc486a78483d159
has 56 uploads, with https://github.com/python-trio/trio/actions/runs/12392561462 and https://github.com/python-trio/trio/actions/runs/12392781287 being the two runs for it.

Analyzing the list of uploads on codecov.io it appears that macOS pypy-3.10 and windows 3.12 x64 got uploaded twice in the latter (so a total of three times), which is what brings the total from 54 to 56. Looking through the log I'm not seeing any indications why it got uploaded twice, maybe it's a codecov-action@v3 bug /shrug

Running codecov twice is a waste, but the root cause there is running the whole CI twice needlessly - so nothing to fix on the codecov end for this one.

[jakkdl]

After switching primary branch from master to main we need to set the default branch in codecov.yml: https://docs.codecov.com/docs/codecov-yaml#default-branch
This is primarily for pages such as https://app.codecov.io/gh/python-trio/trio which currently defaults to master and makes it seem like we haven't uploaded coverage the past three months.

(it sets the default branch on first upload, so if you manually change it you need to do the same on codecov)

[webknjaz]

@jakkdl oh, I was just about to file an issue about that :)

This does not need to go into the config. Instead, I just changed it on the web UI @ https://app.codecov.io/gh/python-trio/trio/config/general.

It should be good now.

[CoolCat467]

Potentially reason why there are duplicate uploads, I've noticed that for pull requests that are branches of https://github.com/python-trio/trio it runs CI both for that branch and for being a pull request

[webknjaz]

@jakkdl

macOS pypy-3.10 [..] got uploaded twice

Yeah, I see 3 uploads for that commit. One of them is from https://github.com/python-trio/trio/actions/runs/12392561462 and then, two are from https://github.com/python-trio/trio/actions/runs/12392781287.

Looking into https://github.com/python-trio/trio/actions/runs/12392781287/job/34592683971, there were no reruns. So I'd suspect a bug in the action (IIRC v3 uses https://github.com/codecov/uploader written in TypeScript) — but there are no logs of retries corroborating this. Or, there's a bug within the Codecov platform where they duplicated the entry somehow.

I'll try looking into the uploaded payloads to see if they're identical or not.

@CoolCat467

Potentially reason why there are duplicate uploads, I've noticed that for pull requests that are branches of python-trio/trio it runs CI both for that branch and for being a pull request

In this instance, the two workflow runs were triggered by two events — merge_group and push. The merge_group event is triggered within GitHub sometime after a human clicked that Merge when ready button — it's a pre-merge check, where the test suite is being run against a temporary merge commit of one or multiple PRs together into main. If it succeeds, the main branch is fast-forwarded to that temporary merge commit and the push event occurs within GitHub, and so it triggers a new workflow run again (if configured this way, which it is, in case of trio).
Both of those workflow runs uploaded coverage information.

Oh, and before these events, whenever a PR is updated (but is not yet ready for merge), a pull_request event is triggered, those runs also upload coverage to Codecov. Codecov knows how to tell these apart because the PR context is being sent over with the coverage data.

[webknjaz]

I'll try looking into the uploaded payloads to see if they're identical or not.

I didn't actually have to download them. I checked that the Download links for both entries coming from push are actually the same — https://api.codecov.io/upload/gh/python-trio/trio/download?path=shelter/v4/github/python-trio/trio/4f66fabf5ca4c2dd40e4e9b76bc486a78483d159/c4cc7127-8a04-4265-90e6-9e422d666f9a.txt.

This means that Codecov's DB displays to upload entries for the same upload. It's their bug.

[webknjaz]

Reported @ codecov/feedback#613.

[webknjaz]

Experimenting with typical configs from other projects: #3158.

[webknjaz]

@webknjaz I also noticed that coverage published from my fork's push event also got uploaded with v5: https://github.com/webknjaz/trio/actions/runs/12404982112/job/34631062081#step:9:246.

I'll try downgrading to v3 to see if that persists.

[jakkdl]

we should probably bump codecov to v5 sooner rather than later if we're going to play around this much with it, so we don't start reporting v3-only bugs and stuff

[webknjaz]

more fun stuff:

• Bump Codecov action to v5 #3161
• 🧪 Publish MyPy type coverage to Codecov #3162

[jakkdl]

another upload fail https://github.com/python-trio/trio/actions/runs/12436356589/job/34724074506#step:6:43

[2024-12-20T18:15:13.830Z] ['error'] There was an error running the uploader: Error uploading to https://codecov.io: Error: getaddrinfo EAI_AGAIN codecov.io
Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v3/dist/codecov' failed with exit code 255

[jakkdl]

two more upload fails today. I almost wanna disable strict errors, but that could give hard-to-explain coverage drops, soooo

fuck codecov 🙃

[A5rocks]

Given we have 100% coverage now ™️, we could actually ditch codecov once and for all.

But requiring 100% coverage on every PR is a bit much. So IMO no.

---

Is there a reason we need to care about coverage drops? If I see that a PR dropped 10% or something, I can see on codecov's site the differences and (being human) I can see whether there's actually an issue. I don't think it actually blocks merge queue -- if it does, maybe there's some other workaround? I see e.g. latest commit on main had failing CI due to a codecov failure and that renders merge queue seemingly useless.

Basically, I propose disabling strict errors and accepting that codecov only provides a general idea. With a workaround to avoid codecov in merge queue if it actually blocks that (i.e. just skipping the codecov skip based on the GitHub actions event type).

[jakkdl]

Given that codecov is failing, I agree with that. I think it should be a strong expectation that you should have 100% coverage when submitting a PR - because it's often much easier&faster for the author to write tests for their own code than for another person to do it.

But long-term I think the fix should be to require 100% coverage, but we check that "locally". Either by combining all the runs ourselves in an action, or with something like coverage-conditional-plugin #3163 (comment)