Skip to content

Add Codecov integration for test coverage tracking #110

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

Draft
wants to merge 1 commit into
base: unstable
Choose a base branch
from

Conversation

ivinjabraham
Copy link
Contributor

This PR adds Codecov integration to track code coverage. With this setup, Codecov will automatically analyze test coverage and post comments on PRs, helping us monitor changes in coverage over time. I believe this will be a good incentive to increase our coverage as well.

Requirements

Codecoverage needs a token from their their website. This needs to be put in GitHub secrets as it's referenced in the .yml file as ${{ secrets.CODECOV_TOKEN }}

@davidbtadokoro
Copy link
Collaborator

Thanks for bringing up the idea! Like in #109, code coverage is a great idea, and patch-hub currently lacks a solution for this. In kworkflow, we also used Codecov to host the results (generated by kcov, in this case), so I like the idea.

I will tinker a little bit with cargo tarpaulin before considering merging this, as I am not familiar with this solution. I imagine that you chose the "best one", but I just need some time before considering expanding the CI workflows.

Once again, thanks for the marvelous work!

Copy link
Collaborator

@davidbtadokoro davidbtadokoro left a comment

Choose a reason for hiding this comment

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

Hey @ivinjabraham. I did some research both on tarpaulin and in the workflow in general.

In a nutshell, it seems that tarpaulin is a good choice, and the workflow does the right steps on the right triggers. I left some inline review comments, so please see if you can address them. Feel free to ping me in case of any doubt.

Copy link
Collaborator

@davidbtadokoro davidbtadokoro left a comment

Choose a reason for hiding this comment

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

Hi, @ivinjabraham, and thanks for the update! I think we are getting near to the merging point, but I just need to check things in the kworkflow org level. In the meantime, I added some simple inline comments, and I replied the comment you made from the last review. See if you can address them, please.

In any case, marvelous work!

- Added `.github/workflows/coverage.yml` to automate test coverage
  reports
- Workflows run on every push and PR to `master` and `unstable`
- Uploads data to Codecov

Signed-off-by: Ivin Joel Abraham <[email protected]>
@ivinjabraham
Copy link
Contributor Author

ivinjabraham commented Mar 16, 2025

Just to be clear, with v5 there won't be any upload of commits or anything of the like, right? Ideally, Codecov should only be a report hosted somewhere else to see coverage for pushes/PRs.

Correct, using v5 doesn't upload any of our source code. It's solely for uploading the coverage report. However, to associate reports with different refs, it would upload some git metadata such as commit hashes and branch information.

Reference: https://about.codecov.io/security/#does-codecov-store-source-code

@davidbtadokoro
Copy link
Collaborator

Correct, using v5 doesn't upload any of our source code. It's solely for uploading the coverage report. However, to associate reports with different refs, it would upload some git metadata such as commit hashes and branch information.

Reference: https://about.codecov.io/security/#does-codecov-store-source-code

On the front of uploading code and such, there are no worries. I was talking about CodeCov trying to push a commit into the repo. I don't know if I am mixing things, but I remember that when I tried v4, it failed due to it not being able to push the commit.

About the rest, everything is looking legit, so I'll clear the front about the org, and come back here.

@ivinjabraham
Copy link
Contributor Author

I was talking about CodeCov trying to push a commit into the repo.

AFAIK, this should never happen.

@davidbtadokoro
Copy link
Collaborator

Hey @ivinjabraham, unfortunately I still didn't solve the matter about a org-level secret for us to go forward with this PR. Everything else is legit, though.

@ivinjabraham
Copy link
Contributor Author

Thanks for letting me know and no worries! This isn't needed immediately.

@davidbtadokoro davidbtadokoro marked this pull request as draft May 5, 2025 15:14
@davidbtadokoro
Copy link
Collaborator

Hi @ivinjabraham. I put this PR as a draft for the time being, as this isn't urgent ATM.

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.

2 participants