search

python-trio / trio

Trio – a friendly Python library for async concurrency and I/O
https://trio.readthedocs.io
Other
6.12k stars 335 forks source link

Update to `codecov-action@v4` #2951

Open CoolCat467 opened 7 months ago

CoolCat467 commented 7 months ago

This pull request updates the code coverage action to v4

Reading the release details, it looks like they update node runtime thing, drop support for not telling them the token to use (trio tells them the token so that's fine), and "various argument changes"

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.57%. Comparing base (0090581) to head (bb5c52c).

:exclamation: There is a different number of reports uploaded between BASE (0090581) and HEAD (bb5c52c). Click for more details.

HEAD has 114 uploads less than BASE | Flag | BASE (0090581) | HEAD (bb5c52c) | |------|------|------| |Ubuntu|16|0| |3.8|14|0| |macOS|12|0| |3.11|8|0| |3.12|10|1| |3.10|8|0| |3.9|8|0| |3.13|2|0| |Alpine|2|1| |Windows|28|0| |pypy-3.10|8|0|
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2951 +/- ## ========================================== - Coverage 99.60% 92.57% -7.03% ========================================== Files 121 121 Lines 17882 17882 Branches 3214 3163 -51 ========================================== - Hits 17811 16554 -1257 - Misses 50 1214 +1164 - Partials 21 114 +93 ``` [see 55 files with indirect coverage changes](https://app.codecov.io/gh/python-trio/trio/pull/2951/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio)
A5rocks commented 7 months ago

Surprised at the loss of 3 percentage points of coverage -- is there some platform where things broke?

... interesting: https://github.com/python-trio/trio/actions/runs/7865327122/job/21458126980?pr=2951#step:7:12 (edit: reported to https://github.com/codecov/codecov-action/issues/1279)

A5rocks commented 7 months ago

Is codecov just consistently reporting less coverage? It looks like -3% coverage on all platforms.

It looks like codecov is regenerating coverage.xml rather than using the one we made (which has all the configuration we apply).

A5rocks commented 7 months ago

I tried a workaround and it didn't work. I'm not entirely sure my hypothesis on why this is taking away coverage is correct. But nonetheless consider me displeased that somehow codecov managed to have 2 changes that affect us in a single major version bump >:[

Here's the logs I find suspicious:

On alpine (v3 action):

[2024-02-13T07:09:43.431Z] ['info'] Searching for coverage files...
[2024-02-13T07:09:43.480Z] ['info'] Warning: Some files located via search were excluded from upload.
[2024-02-13T07:09:43.481Z] ['info'] If Codecov did not locate your files, please review https://docs.codecov.com/docs/supported-report-formats
[2024-02-13T07:09:43.481Z] ['info'] => Found 1 possible coverage files:
  coverage.xml
[2024-02-13T07:09:43.481Z] ['info'] Processing empty/coverage.xml...

On macos (v4 action):

warning - 2024-02-13 07:08:53,180 -- No config file could be found. Ignoring config.
warning - 2024-02-13 07:08:53,199 -- No swift data found.
warning - 2024-02-13 07:08:53,214 -- No gcov data found.
info - 2024-02-13 07:08:53,215 -- Generating coverage.xml report in /Users/runner/work/trio/trio/empty
info - 2024-02-13 07:08:57,046 -- Wrote XML report to coverage.xml
info - 2024-02-13 07:08:57,092 -- Found 1 coverage files to upload
info - 2024-02-13 07:08:57,092 -- > empty/coverage.xml

Alright, after that latest workaround attempt:

jakkdl commented 7 months ago

the alpine v3 codecov action is also saying

[2024-02-13T07:23:32.835Z] ['info'] Error fetching git root. Defaulting to /__w/trio/trio/empty. Please try using the -R flag. Error: Error running external program: Error: spawnSync git ENOENT

https://github.com/python-trio/trio/actions/runs/7883042498/job/21509252192?pr=2951#step:7:25

though that's not new (v3 on non-alpine never said it), but maybe also an indication of directory trouble?

does exclude_lines not effect the coverage.xml file but rather how codecov interprets it?

I think this is the case, since it's under the tool.coverage.report heading - whereas tool.coverage.run is for the xml/binary .coverage files. (some local testing suggests the same, modifying the report section after having generated coverage data changes the output of coverage report).

So this is about codecov not being able to find the pyproject.toml file, maybe it's because of us messing around with empty/ and $INSTALLDIR and stuff?

jakkdl commented 7 months ago

Random idea: I realized we don't have a codecov.yaml, which is the codecov-specific configuration file. You don't need one, but maybe it would help resolve our problems https://docs.codecov.com/docs/codecov-yaml

jakkdl commented 5 months ago

@CoolCat467 updating the branch with a merge commit will ping everybody subscribed to a PR (which might be a lot, since joining the trio/ org automatically makes you watch the repository for all activity). AFAIK there's no way to disable merge-commit-notifications in particular, so even though it sometimes is useful it'd be lovely if you refrained from repeatedly bumping PRs unless there's relevant changes or something <3

CoolCat467 commented 5 months ago

@CoolCat467 updating the branch with a merge commit will ping everybody subscribed to a PR (which might be a lot, since joining the trio/ org automatically makes you watch the repository for all activity). AFAIK there's no way to disable merge-commit-notifications in particular, so even though it sometimes is useful it'd be lovely if you refrained from repeatedly bumping PRs unless there's relevant changes or something <3

Sorry, I just want to make sure things stay up to date, but yes I can do that.

CoolCat467 commented 2 months ago

Ok last update was 3 months ago, and while there are not merge conflicts I think that it's reasonable to update it again now that this much time has passed.

CoolCat467 commented 1 month ago

It's been a long time, what exactly was the issue for not using v4 again? Loss compared to previous system or something?

A5rocks commented 1 month ago

Yeah, the issue was decreased coverage despite the coverage files and coverage config not actually changing.

jakkdl commented 1 month ago

https://github.com/codecov/codecov-action/issues/1279 has been resolved, lets see if we can get this working or there are other issues blocking us

jakkdl commented 1 month ago

Current status: