ROSAENG-60112 | test: add tests for delete cluster command

[olucasfreitas]

PR Summary

Add focused unit tests for the delete cluster command at cmd/dlt/cluster/, covering the core business logic in handleClusterDelete and the cleanup command builder buildCommands.

Detailed Description of the Issue

The delete cluster command had 0% test coverage. This is one of the most critical user-facing workflows in the ROSA CLI. The command handles cluster state validation, deletion via the OCM API, and generating cleanup instructions for STS resources (operator roles and OIDC providers).

This PR tests the two internal functions that contain the actual decision logic, without needing to trap os.Exit from the legacy Run: run Cobra pattern.

Related Issues and PRs

• Jira: ROSAENG-60112
• Related PR(s): coverage was addressed in ROSAENG-60112 | test: add focused coverage for untested pkg packages #3277

Type of Change

• feat
• fix
• docs
• style
• refactor
• test
• chore
• build
• ci
• perf

Previous Behavior

cmd/dlt/cluster/ had no automated test coverage.

Behavior After This Change

Six focused test cases covering:

handleClusterDelete (4 tests):

• Cluster already uninstalling: returns nil, logs info
• Happy path: state is ready, DeleteCluster succeeds, returns nil
• GetClusterState API error: propagated
• DeleteCluster API error: propagated

buildCommands (2 tests):

• Non-reusable OIDC config: commands use -c <clusterID>
• Reusable OIDC config: commands use --prefix and --oidc-config-id

How to Test (Step-by-Step)

Preconditions

• Go toolchain installed
• Repository cloned with hooks installed

Test Steps

1. go test ./cmd/dlt/cluster/... -count=1 -v
2. go vet ./cmd/dlt/cluster/...

Expected Results

• 6/6 specs pass
• go vet clean

Proof of the Fix

• Pre-push checks passed (format, build, lint, coverage, unit tests)

Breaking Changes

• No breaking changes

Developer Verification Checklist

• Commit subject/title follows [JIRA-TICKET] | [TYPE]: <MESSAGE>.
• PR description clearly explains both what changed and why.
• Relevant Jira/GitHub issues and related PRs are linked.
• make install-hooks has been run in this clone.
• Tests were added/updated where appropriate.
• I manually tested the change.
• make test passes.
• make lint passes.
• make rosa passes.
• Documentation or repo-local agent guidance was added/updated where appropriate.
• Any risk, limitation, or follow-up work is documented.

Summary by CodeRabbit

• Tests
  • Added a Ginkgo/Gomega test suite covering cluster deletion behavior, including already-uninstalling, successful deletion, and error handling for failures retrieving cluster state and forbidden delete responses.
  • Added test coverage for generated CLI flags, validating correct argument composition for reusable vs non-reusable OIDC configuration scenarios.
  • Introduced a Ginkgo suite entry point to run the cluster deletion specs.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: d8dd5712-4222-4452-a8eb-d5572dae606f

📥 Commits

Reviewing files that changed from the base of the PR and between a2de374 and 5faaf6c.

📒 Files selected for processing (2)

• cmd/dlt/cluster/cluster_suite_test.go
• cmd/dlt/cluster/cmd_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• cmd/dlt/cluster/cluster_suite_test.go
• cmd/dlt/cluster/cmd_test.go

---

📝 Walkthrough

Walkthrough

A Ginkgo/Gomega test suite was added for cmd/dlt/cluster. It adds a suite entrypoint, covers handleClusterDelete for uninstalling, ready, best-effort, and error cases, and verifies buildCommands flag output for reusable and non-reusable OIDC configs.

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality	⚠️ Warning	FAIL: cmd/dlt/cluster/cmd_test.go uses several bare Expect(err).NotTo(HaveOccurred())/To(HaveOccurred()) assertions without diagnostic messages.	Add meaningful messages to the bare assertions (especially error checks); cleanup via the test runtime is already handled.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, follows the required ticket/type format, and accurately summarizes the main change.
Description check	✅ Passed	The description follows the repository template and includes the issue, change summary, testing steps, and validation details.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All Ginkgo titles in cmd/dlt/cluster are static, descriptive strings; no interpolation, timestamps, IDs, or generated values appear in test names.
Microshift Test Compatibility	✅ Passed	These are unit tests for OCM-backed logic, with no MicroShift-unsupported OpenShift APIs, namespaces, or runtime guards needed.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	These are mock-based unit tests for cmd/dlt/cluster; they use no node/topology assumptions or SNO-skipping logic, so the SNO check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	Only test files were added in cmd/dlt/cluster; no manifests, controllers, or scheduling constraints were modified.
Ote Binary Stdout Contract	✅ Passed	PR only adds tests and a minimal RunSpecs entrypoint; no new process-level stdout writes or stdout-emitting suite config were introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	These are unit tests with mocks only; no IPv4 literals, IP parsing, host formatting, or external network calls were found.
No-Weak-Crypto	✅ Passed	The changed cluster tests and package scan show no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret-comparison usage.
Container-Privileges	✅ Passed	The PR only adds Go test files; no container/K8s manifests or privileged settings (privileged, hostPID, allowPrivilegeEscalation, SYS_ADMIN, runAs*) were introduced.
No-Sensitive-Data-In-Logs	✅ Passed	PR adds tests only; the new specs assert generic messages and contain no logging or secret/PII strings.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[amandahla]

You can safely override the ci/prow/security for now.

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@cmd/dlt/cluster/cmd_test.go`:
- Around line 78-96: The test in handleClusterDelete only covers the success
path and does not verify that the delete request carries bestEffort through.
Update the cmd/dlt/cluster/cmd_test.go spec around handleClusterDelete to
inspect the outgoing delete call from the ApiServer mock and assert it includes
best_effort=true, using the existing clusterReady, clusterId, and bestEffort
flow so the test validates the pass-through contract rather than just a happy
path.

🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: b7a566a4-60ab-4f76-a3df-8841130e5cbf

📥 Commits

Reviewing files that changed from the base of the PR and between 2ece616 and efd5acf.

📒 Files selected for processing (2)

• cmd/dlt/cluster/cluster_suite_test.go
• cmd/dlt/cluster/cmd_test.go

[olucasfreitas]

/override ci/prow/security

[openshift-ci]

@olucasfreitas: Overrode contexts on behalf of olucasfreitas: ci/prow/security

Details

In response to this:

/override ci/prow/security

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 kubernetes-sigs/prow repository.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.14%. Comparing base (9c0c55a) to head (5faaf6c).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3298      +/-   ##
==========================================
+ Coverage   27.02%   27.14%   +0.11%     
==========================================
  Files         334      334              
  Lines       36704    36704              
==========================================
+ Hits         9920     9964      +44     
+ Misses      26029    25985      -44     
  Partials      755      755              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[amandahla]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amandahla, olucasfreitas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [amandahla,olucasfreitas]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment