OCPBUGS-72571: remove "OperatorHub" from OLM integration tests where … #15911
Conversation
openshift-ci-robot
added
jira/severity-moderate
|
@rhamilto: This pull request references Jira Issue OCPBUGS-72571, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
WalkthroughTests and a view helper were updated for naming consistency: capitalization and terminology changes in multiple Cypress tests, plus a parameter/property rename from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@rhamilto: This pull request references Jira Issue OCPBUGS-72571, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
@rhamilto: This pull request references Jira Issue OCPBUGS-72571, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.ts:
- Line 115: The test title string in the it(...) call currently contains a typo
"deprectaed"; update the description to "verify deprecated version warnings in
the Operator details panel" by editing the it('verify deprectaed version
warnings in the Operator details panel', ...) invocation to correct "deprectaed"
→ "deprecated" so the test name reads "verify deprecated version warnings in the
Operator details panel".
In
@frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-global.cy.ts:
- Around line 4-7: The test fails because operator.install still expects the old
parameter name operatorHubCardTestID; update the operator.install function
signature in operator.view.ts to replace operatorHubCardTestID with
operatorCardTestID, and update any internal references (e.g., the
cy.byTestID(...) call) to use operatorCardTestID so the call
operator.install(testOperator.name, testOperator.operatorCardTestID, ...)
matches the function parameters.
In
@frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-single-namespace.cy.ts:
- Around line 6-10: The install() signature in operator.view.ts still uses the
old parameter name operatorHubCardTestID; update its parameter list to replace
operatorHubCardTestID with operatorCardTestID and keep the rest unchanged
(operatorName: string, operatorCardTestID: string, installToNamespace: string =
GlobalInstalledNamespace, useOperatorRecommendedNamespace: boolean = false) so
the view layer matches the renamed test property used by
operator-install-single-namespace.cy.ts, operator-install-global.cy.ts and
operator-uninstall.cy.ts.
🧹 Nitpick comments (1)
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.ts (1)
191-241: LGTM with a minor note on capitalization consistency.The test logic and assertions are correct. However, these test descriptions use lowercase "operator" (e.g., "deprecated operator warnings") while earlier tests use "Operator" with a capital O (e.g., "deprecated Operator warnings" on lines 55, 77, 141, 162). Consider aligning capitalization across all test descriptions for consistency, though this is a nitpick.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (8)
frontend/packages/integration-tests-cypress/tests/app/auth-multiuser-login.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/catalog-source-details.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/edit-default-sources.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-hub.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-global.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-single-namespace.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-uninstall.cy.ts
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/packages/integration-tests-cypress/tests/app/auth-multiuser-login.cy.ts (1)
frontend/packages/integration-tests-cypress/views/nav.ts (1)
nav(6-105)
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.ts (1)
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/views/operator.view.ts (1)
operator(11-214)
🔇 Additional comments (10)
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.ts (5)
55-75: LGTM!Test description properly updated from "OperatorHub tile" to "Operator tile" — aligns with the broader terminology normalization effort. Test logic remains intact and correctly verifies the deprecated badge on the catalog tile.
77-88: LGTM!Terminology updated to "Operator details panel" — consistent with the PR's goal. Test assertions for the deprecated badge and package warning message are appropriate.
90-113: LGTM!Description updated appropriately. The test correctly exercises the channel selection flow and verifies deprecation warning icons and messages appear as expected.
141-160: LGTM!Test description and log messages updated to reflect "Install Operator details page" terminology. The assertions correctly verify all three deprecation warning types (package, channel, version) on the subscription page.
162-189: LGTM!Terminology normalized in both the test description and log messages. The test flow correctly exercises the full installation approval path and verifies the deprecated badge appears on the Installed Operators page.
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/catalog-source-details.cy.ts (1)
26-27: LGTM - Capitalization consistency fix.The comment now correctly capitalizes "OperatorHub" as a proper noun, consistent with the selector on line 24 and the section header assertion on line 27.
Side note: The pre-existing typo "verfiy" remains on lines 26, 44, and 85 — consider fixing those in a follow-up if you're tidying up comments.
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-hub.cy.ts (1)
3-3: LGTM - Test descriptions updated to match new terminology.The suite and test descriptions are updated consistently:
- Suite: "Interacting with Operators" accurately describes the test scope
- Test: "Operator catalog items" aligns with the actual test behavior
The test assertions (line 20:
'Software Catalog'heading) and navigation remain consistent with the updated terminology across the PR.Also applies to: 17-17
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/edit-default-sources.cy.ts (1)
18-25: LGTM - Capitalization standardized for OperatorHub references.Good judgment here — this test specifically exercises the OperatorHub configuration/details page, so retaining "OperatorHub" in the descriptions is appropriate. The capitalization is now consistent with:
- The legacy test ID selector:
cy.byLegacyTestID('OperatorHub')(line 22)- The section header assertion:
'OperatorHub details'(line 25)Same "verfiy" typo note as the other file — a quick find/replace would clean those up across the test suite.
frontend/packages/integration-tests-cypress/tests/app/auth-multiuser-login.cy.ts (1)
44-45: Navigation item name is correctly updated to "Software Catalog".The test assertion is accurate. The console-extensions.json confirms this nav item is defined with
"section": "ecosystem"and"perspective": "admin", making it an admin-only section. The change from "OperatorHub" to "Software Catalog" reflects the actual current UI naming across all locales (EN, JA, ZH, FR, ES), with backward compatibility maintained through path routing that still recognizes the old "operatorhub" paths. The test logic correctly validates that non-admin users lack access to this admin-scoped navigation item.frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-uninstall.cy.ts (1)
6-10: Rename applied correctly in both definition and usage.The
operatorCardTestIDproperty rename (line 8) and its corresponding usage inoperator.install()(line 30) are consistent. The remainder of the test logic—including the uninstall modal interactions, intercepts, and operand lifecycle assertions—is unaffected by this terminology change.Also applies to: 28-32
...perator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.ts
Outdated
Show resolved
Hide resolved
| const testOperator = { | ||
| name: 'Data Grid', | ||
| operatorHubCardTestID: 'operator-Data Grid', | ||
| operatorCardTestID: 'operator-Data Grid', | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for remaining operatorHubCardTestID references (deprecated)
echo "=== Checking for remaining 'operatorHubCardTestID' references ==="
rg 'operatorHubCardTestID' --type ts || echo "✓ No deprecated references found"
echo ""
echo "=== Verifying 'operatorCardTestID' usage across test files ==="
rg 'operatorCardTestID' --type ts -l
echo ""
echo "=== Checking operator.install() signature in operator.view ==="
fd 'operator.view.ts' -x head -100 {}Repository: openshift/console
Length of output: 5856
Parameter name mismatch breaks the test—operator.view.ts signature must be updated.
The refactor is incomplete. While this file correctly renamed the property from operatorHubCardTestID to operatorCardTestID, the operator.install() function in operator.view.ts still expects the old parameter name operatorHubCardTestID. The test will fail when calling operator.install(testOperator.name, testOperator.operatorCardTestID, ...) because the function signature hasn't been updated to match.
Update the operator.install() signature and its usages in operator.view.ts to use operatorCardTestID instead of operatorHubCardTestID. This affects the function parameter declaration and the internal cy.byTestID(operatorCardTestID) call.
🤖 Prompt for AI Agents
In
@frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-global.cy.ts
around lines 4 - 7, The test fails because operator.install still expects the
old parameter name operatorHubCardTestID; update the operator.install function
signature in operator.view.ts to replace operatorHubCardTestID with
operatorCardTestID, and update any internal references (e.g., the
cy.byTestID(...) call) to use operatorCardTestID so the call
operator.install(testOperator.name, testOperator.operatorCardTestID, ...)
matches the function parameters.
...or-lifecycle-manager/integration-tests-cypress/tests/operator-install-single-namespace.cy.ts
Show resolved
Hide resolved
openshift-ci
bot
added
component/olm
rhamilto
force-pushed
the
OCPBUGS-72571
branch
from
610771a to
2d8f225
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/edit-default-sources.cy.ts (1)
18-25: Capitalization standardization looks good.The terminology changes correctly capitalize "OperatorHub" as a proper noun, consistent with OpenShift product naming conventions. The test title and log message now align with how this feature is referenced elsewhere in the UI.
One minor nit: line 24 has a pre-existing typo "verfiy" → "verify". Since you're already touching this area, might be worth fixing opportunistically.
📝 Optional typo fix
- // verfiy OperatorHub details page is open + // verify OperatorHub details page is open
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
frontend/packages/integration-tests-cypress/tests/app/auth-multiuser-login.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/catalog-source-details.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/edit-default-sources.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-hub.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-global.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-single-namespace.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-uninstall.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests-cypress/views/operator.view.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/packages/integration-tests-cypress/tests/app/auth-multiuser-login.cy.ts
- frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-global.cy.ts
- frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/deprecated-operator-warnings.cy.ts
- frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/catalog-source-details.cy.ts
- frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-hub.cy.ts
🔇 Additional comments (5)
frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-uninstall.cy.ts (2)
6-10: LGTM – test object aligns with updatedinstallsignature.The
testOperatorproperty rename tooperatorCardTestID(line 8) correctly reflects the terminology change and matches the updatedoperator.installparameter name.
28-32: LGTM – install call correctly references renamed property.The
operator.installinvocation now passestestOperator.operatorCardTestID(line 30), properly aligned with both the updated function signature and the object property rename. No functional change, just naming consistency.frontend/packages/operator-lifecycle-manager/integration-tests-cypress/tests/operator-install-single-namespace.cy.ts (2)
6-10: LGTM – consistent rename across test fixtures.The
operatorCardTestIDproperty (line 8) follows the same pattern as other test files in this PR. Good consistency.
35-39: LGTM – install invocation correctly updated.The
operator.installcall (line 37) properly uses the renamedtestOperator.operatorCardTestID. The test flow is unchanged; only the identifier naming is standardized.frontend/packages/operator-lifecycle-manager/integration-tests-cypress/views/operator.view.ts (1)
12-23: Clean parameter rename with full caller coverage.The
operatorHubCardTestID→operatorCardTestIDrename is properly implemented and consistent. All three call sites have been updated:operator-install-global.cy.ts,operator-install-single-namespace.cy.ts, andoperator-uninstall.cy.tscorrectly passtestOperator.operatorCardTestID. No stale references remain.
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/assign @TheRealJon |
|
/retest |
1 similar comment
|
/retest |
|
The text changes look good to me and pass in CI |
openshift-ci-robot
added
the
verified
|
@yapei: 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. |
|
/label acknowledge-critical-fixes-only |
openshift-ci
bot
added
the
acknowledge-critical-fixes-only
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, rhamilto 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 |
|
@rhamilto: 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. |
|
@rhamilto: Jira Issue Verification Checks: Jira Issue OCPBUGS-72571 Jira Issue OCPBUGS-72571 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
Fix included in accepted release 4.22.0-0.nightly-2026-01-19-085729 |
6 participants
…appropriate
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.