chore: move coverage report copying logic to makefile target#54
chore: move coverage report copying logic to makefile target#54nilushancosta merged 1 commit intoopenchoreo:mainfrom
Conversation
Signed-off-by: Nilushan Costa <nilushan@wso2.com>
📝 WalkthroughWalkthroughThe PR removes coverage file handling steps from CI and PR workflows, shifting responsibility to individual module Makefiles that now generate module-prefixed coverage artifacts in the parent directory. Additionally, Helm chart versions are bumped across three observability modules. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
observability-tracing-openobserve/Makefile (1)
17-19: Consider handling the case where no coverage file is generated.If there are no test files or
go testfails to producecoverage.out, themvcommand will fail. While this surfaces configuration issues, you may want to make it more resilient:💡 Optional: Add existence check
unit-test: go test -coverprofile=coverage.out ./... - mv coverage.out ../$(MODULE_NAME)-coverage.out + [ -f coverage.out ] && mv coverage.out ../$(MODULE_NAME)-coverage.out || trueThis is optional since failing loudly when coverage is expected but missing can also be a valid design choice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@observability-tracing-openobserve/Makefile` around lines 17 - 19, The Makefile unit-test target currently runs "go test -coverprofile=coverage.out ./..." then unconditionally moves coverage.out with "mv coverage.out ../$(MODULE_NAME)-coverage.out", which will fail if no coverage file is produced; update the unit-test recipe (the target named unit-test) to check for the existence and/or success before moving—e.g., only run the mv when coverage.out exists (or check the exit status of go test) so the move is skipped (or a clear message emitted) when coverage.out is not generated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@observability-tracing-openobserve/Makefile`:
- Around line 17-19: The Makefile unit-test target currently runs "go test
-coverprofile=coverage.out ./..." then unconditionally moves coverage.out with
"mv coverage.out ../$(MODULE_NAME)-coverage.out", which will fail if no coverage
file is produced; update the unit-test recipe (the target named unit-test) to
check for the existence and/or success before moving—e.g., only run the mv when
coverage.out exists (or check the exit status of go test) so the move is skipped
(or a clear message emitted) when coverage.out is not generated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc632759-f966-44c4-98d7-9bd5c27a770a
📒 Files selected for processing (11)
.github/workflows/ci.yaml.github/workflows/pr.yamlobservability-logs-openobserve/Makefileobservability-logs-openobserve/README.mdobservability-logs-openobserve/helm/Chart.yamlobservability-metrics-prometheus/Makefileobservability-metrics-prometheus/README.mdobservability-metrics-prometheus/helm/Chart.yamlobservability-tracing-openobserve/Makefileobservability-tracing-openobserve/README.mdobservability-tracing-openobserve/helm/Chart.yaml
💤 Files with no reviewable changes (2)
- .github/workflows/ci.yaml
- .github/workflows/pr.yaml
Purpose
openchoreo/openchoreo#2755
Update the unit-test target it the Makefile to copy the generated code coverage report to the repository root
Goals
Make the code coverage publishing workflow generic so it just needs to run
make unit-testApproach
Updated the Makefiles' unit-test target to copy the coverage report to the repository root
Summary by CodeRabbit