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 coverage build and upload unit-test coverage to codecov.io #268

Merged
merged 3 commits into from
Apr 25, 2023

Conversation

samcunliffe
Copy link
Member

@samcunliffe samcunliffe commented Apr 25, 2023

Context/Description

Adds a coverage build to CMakeLists and additions to the GitHub action to upload it all to codecov.io.

More details

  • Adds a coverage build to CMakeLists. Currently, this is only for the tdms_tests executable. Could also add it to tdms itself if we wanted to check the system tests' coverage.
  • In our CI GitHub action, I install lcov (for nice summaries of the coverage stats for humans, and for upload to codecov.io).
  • Add a badge 🎉 (but we need to get a token before it will be meaningful).

Testing

Tested over on my fork.

Documentation

  • Added to developers' documentation with a section under "unit testing".

* Solves #120.
* Adds a coverage build to CMakeLists. Currently this is only for the
  `tdms_tests` executable. Could also add it to `tdms` itself if we
  wanted to check the system tests' coverage.
* In the CI GH action, I install lcov (for nice summaries of the
  coverage stats for humans, and for upload to codecov.io).
* Add a badge 🎉 (bit we need to add a token).
@samcunliffe samcunliffe added technical Technical and meta issues, not related to physics but infrastructure. testing Adding or requesting more test coverage housekeeping Code cleanup labels Apr 25, 2023
@samcunliffe samcunliffe added this to the End of ARC project. milestone Apr 25, 2023
README.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@722d461). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #268   +/-   ##
=======================================
  Coverage        ?   47.56%           
=======================================
  Files           ?       63           
  Lines           ?     7804           
  Branches        ?        0           
=======================================
  Hits            ?     3712           
  Misses          ?     4092           
  Partials        ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@samcunliffe samcunliffe changed the title Add coverage build and upload to codecov.io Add coverage build and upload unit test coverage to codecov.io Apr 25, 2023
@samcunliffe samcunliffe changed the title Add coverage build and upload unit test coverage to codecov.io Add coverage build and upload unit-test coverage to codecov.io Apr 25, 2023
Copy link
Collaborator

@willGraham01 willGraham01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing to flag is that we should probably make our stance on Windows-testing a bit clearer, but that's not a bug introduced with these changes.

@@ -43,7 +43,8 @@ jobs:
# also test 'Release'.
build_testing: [ON, OFF]
exclude:
# Currently our tests don't compile in Windows so skip this.
# Currently our tests don't compile, and nor do we have a coverage
# build in Windows so skip this.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬 Should this maybe be an issue or are we committing to not supporting Windows?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to say no. I softly doubt any of Peter's future developers will be using Windows.
Could tack this on to...

...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a completely different tool to gcov, though. Also, I don't know how to run it... maybe Mosè does?

Copy link
Member

@giordano giordano Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what is "It" here? lcov?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows? What's Windows?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍉

@samcunliffe samcunliffe merged commit f6832b2 into main Apr 25, 2023
@samcunliffe samcunliffe deleted the coverage-build branch April 25, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Code cleanup technical Technical and meta issues, not related to physics but infrastructure. testing Adding or requesting more test coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test coverage to CI.
3 participants