OPRUN-4228: Add boxcutter tests#672
Conversation
WalkthroughAdded five new OLMv1 Boxcutter runtime test entries to the test manifest file and implemented corresponding test cases in Go that validate cluster extension lifecycle operations including installation, status updates, resource labeling, cleanup, and reinstallation. All tests are gated by the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
134d3f9 to
d339ccb
Compare
d339ccb to
80524d7
Compare
|
@camilamacedo86: This pull request explicitly references no jira issue. 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. |
|
@camilamacedo86: This pull request references OPRUN-4228 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 "4.22.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. |
80524d7 to
685cfb0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openshift/tests-extension/test/olmv1-boxcutter.go (1)
163-192: Consider adding DeferCleanup for the first extension as a safety net.If this test fails between creating the first ClusterExtension (line 168) and calling
firstCleanup()(line 176), orphaned resources will remain until the next test run cleans them viaEnsureCleanupClusterExtension. While this eventually self-heals, adding aDeferCleanupcall immediately after creation provides more robust cleanup:♻️ Suggested improvement
By("applying the ClusterExtension resource for the first time") firstName, firstCleanup := helpers.CreateClusterExtension(opName, "", nsName, unique, helpers.WithCatalogNameSelector(ccName)) + DeferCleanup(firstCleanup) By("waiting for the first installation to complete")This ensures cleanup happens even if the test fails early. The explicit
firstCleanup()call on line 176 will still work (cleanup functions typically ignore "not found" errors).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openshift/tests-extension/test/olmv1-boxcutter.go` around lines 163 - 192, After creating the first ClusterExtension with helpers.CreateClusterExtension (returning firstName, firstCleanup), immediately register the cleanup with DeferCleanup(firstCleanup) to guarantee resource cleanup if the test fails before the explicit firstCleanup() call; keep the existing explicit firstCleanup() later (it will be a no-op if already cleaned), and reference firstName/firstCleanup/CreateClusterExtension/DeferCleanup when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@openshift/tests-extension/test/olmv1-boxcutter.go`:
- Around line 163-192: After creating the first ClusterExtension with
helpers.CreateClusterExtension (returning firstName, firstCleanup), immediately
register the cleanup with DeferCleanup(firstCleanup) to guarantee resource
cleanup if the test fails before the explicit firstCleanup() call; keep the
existing explicit firstCleanup() later (it will be a no-op if already cleaned),
and reference firstName/firstCleanup/CreateClusterExtension/DeferCleanup when
making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a06ba30c-decf-499d-8332-bb07c1f4ea4b
📒 Files selected for processing (2)
openshift/tests-extension/.openshift-tests-extension/openshift_payload_olmv1.jsonopenshift/tests-extension/test/olmv1-boxcutter.go
|
/test e2e-aws-techpreview-olmv1-ext |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/label qe-approved |
|
@camilamacedo86: This pull request references OPRUN-4228 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 "4.22.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. |
|
@bandrade: This PR has been marked as verified by 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. |
|
@camilamacedo86: 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. |
No description provided.