test(vgpumonitor): add unit tests for metric-sending helpers - #2448
test(vgpumonitor): add unit tests for metric-sending helpers#2448adity1raut wants to merge 1 commit into
Conversation
sendMetric and sendLegacyMetric in cmd/vGPUmonitor/metrics.go had no direct unit test coverage; they were only exercised indirectly through the collector-level Describe/Collect test. Add tests for both the success path and the error path (mismatched label count) of each helper. Signed-off-by: Aditya Raut <araut7798@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: adity1raut The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded tests for ChangesMetric Sender Tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Thanks for the contribution. These tests directly exercise two very small metric-emission wrappers, while the same behavior is already exercised through the collector-level tests. This PR does not address a user-visible defect or a maintainer-confirmed gap, and splitting this small coverage exercise from #2447 creates more review and maintenance overhead than project value. We are therefore closing this PR. If a specific metrics regression is found later, please start from a reproducible failure and keep the corresponding fix and regression test together. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds direct unit tests for
sendMetricandsendLegacyMetricincmd/vGPUmonitor/metrics.go. Both helpers had no direct test coverage — they were only exercised indirectly through the collector-levelDescribe/Collecttest, so thesendMetricerror path (mismatched label count) and the nil-descriptor no-op insendLegacyMetricweren't independently verified.This is a small, standalone piece of the metrics gap analysis discussed in #2126 (test coverage for the existing vGPUmonitor metrics surface before further changes are layered on top). Companion PR for the scheduler side: #2447.
Which issue(s) this PR fixes:
Relates to #2126
Special notes for your reviewer:
Test-only change, no production code touched. Ran locally:
Does this PR introduce a user-facing change?:
AI assistance disclosure: This PR was written primarily by Claude Code (test cases and implementation), based on my own review of the existing metrics.go coverage gaps.
Summary by CodeRabbit