Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧪 Fix uploading coverage from GHA to Codecov #941

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

Jamim
Copy link
Contributor

@Jamim Jamim commented Feb 1, 2024

Hello @webknjaz,

What do these changes do?

The newest version of the codecov-action fails to submit coverage, so it isn't ready for use yet.

These changes downgrade codecov-action to v3.

Are there changes in behavior for the user?

No.

Related issues

Checklist

  • I think the code is well written
  • Documentation reflects the changes

Best regards!

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Feb 1, 2024
@Jamim
Copy link
Contributor Author

Jamim commented Feb 1, 2024

One more remarkable issue:

@Jamim
Copy link
Contributor Author

Jamim commented Feb 1, 2024

🤦🏼 Well, while this PR fixes coverage uploading, some Codecov-related logic marks this PR with ❌

@Jamim
Copy link
Contributor Author

Jamim commented Feb 1, 2024

For the sake of transparency, one more related issue:

@Jamim
Copy link
Contributor Author

Jamim commented Feb 1, 2024

@webknjaz wait a minute, if there's no checkout for the Tests status job, that's why the action can't find the config.

@webknjaz
Copy link
Member

webknjaz commented Feb 2, 2024

@webknjaz wait a minute, if there's no checkout at for the Tests status job, that's why the action can't find the config.

I think this is because Codecov suddenly swapped the defaults without telling anybody:

@webknjaz webknjaz changed the title 🐛 Fix Codecav coverage uploading 🐛 Fix Codecov coverage uploading Feb 2, 2024
@Jamim Jamim force-pushed the fix/codecov branch 2 times, most recently from 70493ea to 0b990c5 Compare February 2, 2024 00:23
@Jamim Jamim requested a review from webknjaz February 2, 2024 00:25
CHANGES/941.contrib.rst Outdated Show resolved Hide resolved
CHANGES/941.contrib.rst Outdated Show resolved Hide resolved
The newest version of the codecov-action fails
to submit coverage, so it isn't ready for use yet.

These changes downgrade codecov-action to v3.

Co-authored-by: Sviatoslav Sydorenko <wk@sydorenko.org.ua>
@Jamim
Copy link
Contributor Author

Jamim commented Feb 2, 2024

I'm not sure what the hell is going on this time, but coverage data is looking weird.
Just compare these two runs:

It counts lines that shouldn't be counted.
I guess it's related to mypy.

@Jamim
Copy link
Contributor Author

Jamim commented Feb 2, 2024

@webknjaz,
After a quick research I came to conclusion that it's mypy who spoils coverage reports.
I don't think it's a wise idea to mix test coverage with type annotation coverage data.
How can we distinct them on Codecov?

@Jamim
Copy link
Contributor Author

Jamim commented Feb 2, 2024

How can we distinct them on Codecov?

Well, I just didn't know about the Flags.

@webknjaz
Copy link
Member

webknjaz commented Feb 2, 2024

Yes, the metrics for flags are combined. The details are somewhat granular, which is enough for now. I'm planning to experiment with combining the coverage setup with https://hynek.me/articles/ditch-codecov-python/ to expose the HTML files and stuff additionally. And maybe, add more stable checks for some code areas.

@webknjaz webknjaz changed the title 🐛 Fix Codecov coverage uploading 🧪 Fix uploading coverage from GHA to Codecov Feb 2, 2024
@webknjaz webknjaz merged commit 19d0511 into aio-libs:master Feb 2, 2024
49 of 51 checks passed
@Jamim Jamim deleted the fix/codecov branch February 2, 2024 03:16
@webknjaz
Copy link
Member

webknjaz commented Feb 2, 2024

Interestingly, the MyPy-produced coverage doesn't include anything from the multidict/ folder. I wonder why.

@webknjaz
Copy link
Member

webknjaz commented Feb 2, 2024

https://github.com/aio-libs/multidict/blob/19d0511/.mypy.ini#L2-L4
well, that explains it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants