test(util): cover MarkAnnotationsToDelete - #2980
Conversation
It wraps GetNode and RemoveNodeAnnotation, both already tested, but its own composition and the node-not-found error path had no direct test. Add one covering the success path and that path. Signed-off-by: Akanksha Trehun <akankshatrehun@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: magic-peach 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: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe change adds tests for ChangesAnnotation deletion test coverage
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: 🔵 Low · up to This test-only change adds coverage for annotation removal and missing-node handling without changing runtime behavior. The missing-node test could accept a different error with similar text, so the coverage is slightly less durable but remains mergeable. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Thanks! But MarkAnnotationsToDelete isn't actually untested... TestRemoveAnnotation (from #2052) already covers both the success and GetNode-not-found paths. |
|
You're right, thanks for catching that. TestRemoveAnnotation from #2052 already covers both branches through MarkAnnotationsToDelete directly, I missed it because the test function isn't named after the function it exercises. Closing this, no new coverage here. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
util.MarkAnnotationsToDeleteis the only exported function inpkg/util/util.gowithout a direct unit test. It's a thin wrapper overGetNodeandRemoveNodeAnnotation, both already tested individually, but its own composition and error-propagation path (whenGetNodefails) were untested. AddsTestMarkAnnotationsToDeleteandTestMarkAnnotationsToDelete_NodeNotFoundfollowing the existing fake-clientset pattern used by the neighboringGetNode/RemoveNodeAnnotationtests.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Test-only change, no production code touched. Ran
go test ./pkg/util/... -run TestMarkAnnotationsToDelete -v --race -count=1, both new tests pass.Does this PR introduce a user-facing change?:
NONE
This PR was written primarily by Claude Code, an AI assistant, under my direction and review. I verified the tests by running them locally and reviewed the diff before submitting.
Summary by CodeRabbit