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

Add code coverage tests #2072

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

SWilson4
Copy link
Member

@SWilson4 SWilson4 commented Feb 7, 2025

This PR demonstrates a simple code coverage testing setup using lcov and coveralls.io, inspired by OpenSSL's coveralls setup. I have integrated my fork with coveralls.io for demonstration purposes: see https://coveralls.io/github/SWilson4/liboqs.

Currently, stateful signatures are excluded from code coverage builds. I did this in order to reduce the inevitable CI trial-and-error time. However, I'm not sure if it makes sense to include them, since we might not want full coverage of the many variants running on every single PR.

TODO before merge:

  • add curl to ci-ubuntu-latest image (dependency for coveralls GitHub actions)
  • decide on final coverage
  • register with coveralls.io (free for public repos)
  • set up necessary environment variables
  • remove references to SWilson4/liboqs / sw-code-coverage and replace them with open-quantum-safe/liboqs / main.

Closes #167.

  • Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
  • Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider will also need to be ready for review and merge by the time this is merged.)

Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
- name: Run lcov
run: lcov -d . -c -o lcov.info --exclude /usr/lib,/usr/include --ignore-errors unused
- name: Upload to coveralls.io
uses: coverallsapp/[email protected]

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 8: third-party GitHubAction not pinned by hash
Click Remediation section below to solve this issue
runs-on: ubuntu-latest
steps:
- name: Finish coveralls.io
uses: coverallsapp/[email protected]

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 8: third-party GitHubAction not pinned by hash
Click Remediation section below to solve this issue
@baentsch
Copy link
Member

baentsch commented Feb 8, 2025

Very nice @SWilson4 ! I just wonder whether it's necessary to run this at every PR: What about doing this in the weekly regression-avoidance check? Then I wonder whether it'd be possible to make the source code accessible via the UI at coveralls.io thus allowing one to see which files get coverage where (not :). Final suggestion for improvement: Can one set up the system such as to report regressions? This (coverage regression) reporting would be the one reason I'd agree with running this at every PR: PRs reducing coverage the should cause CI to fail.

@SWilson4
Copy link
Member Author

Very nice @SWilson4 ! I just wonder whether it's necessary to run this at every PR: What about doing this in the weekly regression-avoidance check? Then I wonder whether it'd be possible to make the source code accessible via the UI at coveralls.io thus allowing one to see which files get coverage where (not :).

If you go to the details for the build associated with this PR, you can see line-by-line details of the coverage. As far as I can tell, it doesn't cover all the source files but only the ones which actually get configured/built (which makes sense, as the coverage data is generated as part of the build process).

Final suggestion for improvement: Can one set up the system such as to report regressions? This (coverage regression) reporting would be the one reason I'd agree with running this at every PR: PRs reducing coverage the should cause CI to fail.

I know coveralls.io can be configured to post a comment in each PR with a report on coverage changes, with the proper token setup. I'll investigate this and see if we can switch it on for a trial run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code coverage
2 participants