-
Notifications
You must be signed in to change notification settings - Fork 74
Enable coverage tracking #210
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
Enable coverage tracking #210
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Huh, that's weird, I was totally expecting that codecov.io upload to fail for lack of a token. @kislyuk did you perhaps set up codecov.io with an org-wide secret? That'd explain why it worked out of the box :). |
cc76d83
to
9350a3e
Compare
No, I did not set up any org-wide access for Codecov. I've set up Codecov in my other repos and I don't think adding a secret is required for public repos. You can just use the codecov GHA and it will comment with the results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine to me but it would be ideal to collect coverage in the course of running regular CI (not a dedicated coverage workflow) to avoid the chance of the test and coverage workflows drifting apart. The coverage upload can be enabled conditionally on only one CI job like this
- uses: codecov/codecov-action@v5
if: ${{matrix.python-version == '3.12' && matrix.os == 'ubuntu-22.04'}}
The problem is not really with the uploads, but rather that the coverage run segfaults on 3.13, and it also requires an instrumented build of the Cython bit... As things are now there'd be quite a few conditionals in the workflow if we tried to merge them, and the C code running in CI would potentially be quite different from a release build. I think some degree of divergence is going to be hard to avoid here, to be honest, but I do think it'd be valuable to collect coverage for the whole test matrix eventually. I'll see what I can do about that. |
9350a3e
to
c756e56
Compare
669d2e9
to
8869883
Compare
- Only run coverage on ubuntu-latest with Python 3.12 (Cython line tracing seems to have some issues with Python 3.13, to be determined) - There are a few paradoxical red spots in the Cython coverage analysis (code that must run for other blocks to be green, but nevertheless showing up as red) but the analysis does give a good indication of where the blind spots are already. - Factor out test setup into its own action with different PKCS#11 platforms to target, to minimise divergence between tests.yml and coverage.yml. Some divergence is unavoidable due to the fact that the coverage run needs an instrumented build of the C extension, and we still want the regular CI run to work with settings that are somewhat close to the actual release build.
8869883
to
fe57001
Compare
I did my best to limit the opportunities for divergence to the absolute minimum by putting the test setup steps into a common action definition. So the only difference is (a) the actual invocation of the test command, and (b) the env vars/compiler directives used to build the C extension with debugging symbols and line tracing support. Especially (b) is hard to avoid: IMHO not testing against a "release-like" build of the extension at all (i.e. one without instrumentation) is more risky than the overhead of keeping two workflows around. |
@kislyuk getting the codecov.io upload to work will require you to add a
CODECOV_TOKEN
repository secret. I've never done that for an org-managed repo before, but the instructions here are hopefully general enough: https://docs.codecov.com/docs/quick-start. If you don't want to use codecov.io for whatever reason, I'm open to alternative suggestions.