-
Notifications
You must be signed in to change notification settings - Fork 247
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
test: add code coverage reporting #1069
Conversation
Add code coverage reporting so that the report is then available to the PR author and reviewers via codecov GitHub app. Signed-off-by: Muyassarov, Feruzjon <[email protected]>
✅ Deploy Preview for kubernetes-sigs-nfd ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
Thanks @fmuyassarov for working on this.
I think integrating it into prow/test-infra (instead of Github actions) would be the way to go, as you pondered
/cc @ArangoGutierrez Just general update: we need to add a secret into the test infra k8s cluster and I'm in progress of reaching out the team responsible for that. Once done, I will submit another patch in the test-infra to add codecov report uploading steps. |
But this PR is not dependent on that |
yes yes. It was just a heads up for Eduardo, because we discussed with you the plan in slack but not here. |
So... Can I lgtm? or wait? |
You can |
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.
/lgtm
LGTM label has been added. Git tree hash: 9ed5916d3f727df86ca25f25154030a913ae89b8
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ArangoGutierrez, fmuyassarov, marquiz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Add code coverage reporting so that the report is then available to the PR author and reviewers via codecov GitHub app.
@marquiz for uploading/reporting the coverage results, we need to run
make test
. We already run themake test
with one of the Prow job and I didn't want to add another one in the form of GitHub action. But GA is the most straightforward way of adding codecov and uploading it. There is an uploader available which can be executed from shell as well, so that can be added into Prow job run section, but for that codecov requires the token. We could store the token as a secret on GitHub but I don't know if there is a way actually for the Prow to ready/fetch that token. At least I have not seen anything like that.With GitHub actions it is not a problem, because we just pass the
${{ secrets.CODECOV_TOKEN }}
and voilà.Do you have any other alternatives in mind or if not shall I go for adding another make test as GitHub action? I also thought that maybe we drop running unit tests from Prow to avoid duplication but Prow runs on much faster environment than what GitHub provides for free.
Fixes: #1033