Add Codecov integration for unit test coverage#84
Conversation
📝 WalkthroughWalkthroughThe pull request introduces Codecov integration to the project. A Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Security ObservationsToken Handling: The workflow correctly uses Action Source: Confirm No other security findings in these configuration and CI/CD additions. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 ☂️ |
Signed-off-by: Dev Kumar <dkuma@redhat.com>
de7d123 to
bdc77af
Compare
shruthis4
left a comment
There was a problem hiding this comment.
Hi Dev,
I think overall we want the report to be hosted for our midstream repo so we know what the coverage is. I would also want to know the coverage we get from examples/openshift/tests . If possible each in separate buckets.
.github/workflows/integration.yaml
Outdated
| - name: Run go unit tests | ||
| run: make unit-test | ||
|
|
||
| - name: Upload unit test coverage to Codecov |
There was a problem hiding this comment.
This is an upstream workflow, and adding it here would lead to merge conflicts and we do not want to propagate this change to upstream as the repo does not have the codecov app setup.
There was a problem hiding this comment.
Great, thanks for the additional context, let me make new changes around this. The workaround will be to create a separate codecov workflow which will rerun the unit tests in order to upload them.
There was a problem hiding this comment.
There is possibility of making this more efficient, but that will require changes to integration workflow where we store the cover.out file as artifact or cache to persist it.
Makefile
Outdated
| @echo "Running unit tests..." | ||
| KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" | ||
| go test $(shell go list ./... | grep -v /e2e) -coverprofile cover.out | ||
| go test $(shell go list ./... | grep -v /e2e) -coverprofile cover-unit.out |
There was a problem hiding this comment.
same comment here, this Makefile comes from upstream
README.md
Outdated
| # Kubeflow Spark Operator | ||
|
|
||
| [](https://github.com/kubeflow/spark-operator/releases) | ||
| [](https://codecov.io/gh/kubeflow/spark-operator) |
There was a problem hiding this comment.
shouldnt the report be hosted for opendatahub-io/spark-operator fork? Also this README.md is upstream file we inherit, so we should keep this clean and mergable
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/integration.yaml:
- Around line 119-124: The workflow step named "Upload unit test coverage to
Codecov" currently uses a mutable tag reference `codecov/codecov-action@v5`;
replace that with the action pinned to the full 40-character commit SHA for the
v5 tag (resolve `v5` to its commit SHA via the GitHub API and substitute
`codecov/codecov-action@<40-char-sha>`). Keep the step name, files, flags, and
token unchanged (files: cover-unit.out, flags: unit, token: ${{
secrets.CODECOV_TOKEN }}) and commit the updated immutable `uses:` string.
In `@Makefile`:
- Around line 165-167: Makefile was changed to produce and read cover-unit.out
(and generates cover.html) but .gitignore still only ignores cover.out; update
.gitignore to ignore the new artifacts by adding cover-unit.out and cover.html
(or replace the single cover.out entry with a more general pattern like
cover*.out and cover*.html) so local coverage files from the Makefile targets
are not shown in git status or accidentally committed; look for entries related
to cover.out and update them accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 490e5928-f258-4f75-9d23-caca60872735
📒 Files selected for processing (4)
.codecov.yml.github/workflows/integration.yamlMakefileREADME.md
…nd e2e-kustomize coverage, addressing PR review feedback Signed-off-by: Dev Kumar <dkuma@redhat.com>
Purpose of this PR
Add Codecov integration to track unit test code coverage for the Spark Operator.
Proposed changes:
.codecov.ymlconfiguration with project/patch coverage status checks, unit flag (with e2e flag scaffolded for future use), and PR comment layout..github/workflows/integration.yamlusingcodecov/codecov-action@v5.codecovbadge to README.Change Category
Rationale
Enables visibility into unit test coverage trends across PRs, helping maintainers identify untested code paths and prevent coverage regressions. The e2e coverage flag is scaffolded for future enablement.
Checklist
Additional Notes
.codecov.ymland can be enabled if required and once e2e tests produce a coverage profile.codecovneeds a baseline to compare test coverage against, which will be generated once this PR is merged tomain.Summary by CodeRabbit