CNTRLPLANE-3900: Add shared cmd/ unit tests for destroy cluster - #9166
CNTRLPLANE-3900: Add shared cmd/ unit tests for destroy cluster#9166mehabhalodiya wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@mehabhalodiya: This pull request references CNTRLPLANE-3900 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds unit tests for destroy command metadata in the agent, KubeVirt, and OpenStack implementations. The tests verify command usage strings and silenced usage output, validate zero-value and explicitly populated agent destroy options, and confirm OpenStack platform-specific destruction returns no error. Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mehabhalodiya 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 |
openshift-ci
Bot
added
area/platform/kubevirt
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/cluster/agent/destroy_test.go (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse distinct names for nested test variables.
Each subtest closure shadows its enclosing
t; rename the closure parameter tottconsistently.
cmd/cluster/agent/destroy_test.go#L24-L31: rename the closure parameter and passtttoNewGomegaWithT.cmd/cluster/kubevirt/destroy_test.go#L23-L30: rename the closure parameter and passtttoNewGomegaWithT.cmd/cluster/openstack/destroy_test.go#L24-L31: rename the closure parameter and passtttoNewGomegaWithT.Proposed change
- t.Run(name, func(t *testing.T) { - g := NewGomegaWithT(t) + t.Run(name, func(tt *testing.T) { + g := NewGomegaWithT(tt)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cluster/agent/destroy_test.go` around lines 24 - 31, Rename each nested subtest closure parameter from t to tt and pass tt to NewGomegaWithT in cmd/cluster/agent/destroy_test.go:24-31, cmd/cluster/kubevirt/destroy_test.go:23-30, and cmd/cluster/openstack/destroy_test.go:24-31; leave the surrounding test logic unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/cluster/agent/destroy_test.go`:
- Around line 24-31: Rename each nested subtest closure parameter from t to tt
and pass tt to NewGomegaWithT in cmd/cluster/agent/destroy_test.go:24-31,
cmd/cluster/kubevirt/destroy_test.go:23-30, and
cmd/cluster/openstack/destroy_test.go:24-31; leave the surrounding test logic
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: dd5a53a6-fa24-4ab0-8efb-a270f8ab2ad0
📒 Files selected for processing (3)
cmd/cluster/agent/destroy_test.gocmd/cluster/kubevirt/destroy_test.gocmd/cluster/openstack/destroy_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9166 +/- ##
==========================================
+ Coverage 44.62% 44.66% +0.04%
==========================================
Files 775 775
Lines 97105 97085 -20
==========================================
+ Hits 43329 43365 +36
+ Misses 50781 50730 -51
+ Partials 2995 2990 -5 see 6 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Three platforms - KubeVirt, OpenStack and Agent have zero shared unit test coverage for cluster destroy Signed-off-by: mehabhalodiya <mehabhalodiya@gmail.com>
0ed9ac2 to
808ffc6
Compare
|
@mehabhalodiya: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Three platforms - KubeVirt, OpenStack, and Agent have zero shared unit test coverage for cluster destroy
What this PR does / why we need it:
3 new test files for zero-coverage destroy paths:
cmd/cluster/kubevirt/destroy_test.go — Tests
NewDestroyCommandwiring (Use, Short, SilenceUsage). KubeVirt's destroy is a thin delegate tonone.DestroyClusterwith no platform-specific logic.cmd/cluster/openstack/destroy_test.go — Tests
NewDestroyCommandwiring +destroyPlatformSpecifics(verifies it's a no-op returning nil). OpenStack has its ownDestroyClusterwith InfraID validation, but the full flow can't be tested via FAKE_CLIENT because the fake client's scheme doesn't includehyperv1.HostedCluster.cmd/cluster/agent/destroy_test.go — Tests
NewDestroyCommandwiring + theDestroyOptionsstruct (zero values and field retention). Agent'sDestroyClusteris a direct delegation tonone.DestroyCluster.Verification:
go testpasses for all three packages,make lint-fixreports 0 issues,go vetis clean.Which issue(s) this PR fixes:
Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-3900
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit