CONSOLE-5409: Add Playwright e2e test for operator lifecycle metadata UI - #16701
CONSOLE-5409: Add Playwright e2e test for operator lifecycle metadata UI#16701perdasilva wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@perdasilva: 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds lifecycle mock factories, an installed-operators page object, and a Playwright e2e spec that installs an OLM subscription, intercepts lifecycle responses, and checks lifecycle badges for compatible, self-support, and incompatible states. ChangesOperator lifecycle metadata E2E
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
frontend/e2e/mocks/operator-lifecycle.ts (1)
1-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicated shapes across type and factory functions.
The phase/platformCompatibility literal shape is declared three times (
LifecycleData,activePhases,expiredPhases), and theschemastring plusversionswrapper is repeated identically in all three factories. This makes it easy for the schema and mock shape to drift out of sync when one call site is updated but not the others.♻️ Proposed consolidation
+type LifecyclePhase = { name: string; startDate: string; endDate: string }; +type PlatformCompatibility = { name: string; versions: string[] }; + export type LifecycleData = { package: string; schema: string; versions?: { name: string; - platformCompatibility?: { name: string; versions: string[] }[]; - phases?: { name: string; startDate: string; endDate: string }[]; + platformCompatibility?: PlatformCompatibility[]; + phases?: LifecyclePhase[]; }[]; }; const toDateStr = (d: Date): string => d.toISOString().slice(0, 10); -const activePhases = (): { name: string; startDate: string; endDate: string }[] => { +const activePhases = (): LifecyclePhase[] => { ... }; -const expiredPhases = (): { name: string; startDate: string; endDate: string }[] => { +const expiredPhases = (): LifecyclePhase[] => { ... }; + +const SCHEMA = 'io.openshift.operators.lifecycles.v1alpha1'; + +const buildLifecycleData = ( + packageName: string, + version: string, + clusterVersions: string[], + phases: LifecyclePhase[], +): LifecycleData => ({ + package: packageName, + schema: SCHEMA, + versions: [ + { + name: version, + platformCompatibility: [{ name: 'openshift', versions: clusterVersions }], + phases, + }, + ], +}); export const makeLifecycleActiveAndCompatible = ( packageName: string, version: string, clusterVersion: string, -): LifecycleData => ({ - package: packageName, - schema: 'io.openshift.operators.lifecycles.v1alpha1', - versions: [ - { - name: version, - platformCompatibility: [{ name: 'openshift', versions: [clusterVersion] }], - phases: activePhases(), - }, - ], -}); +): LifecycleData => buildLifecycleData(packageName, version, [clusterVersion], activePhases());🤖 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 `@frontend/e2e/mocks/operator-lifecycle.ts` around lines 1 - 84, The lifecycle mock shape is duplicated across LifecycleData, activePhases/expiredPhases, and the three factory helpers, so centralize the shared phase/platformCompatibility/version object types and the common schema/versions wrapper. Reuse those shared symbols from operator-lifecycle.ts inside makeLifecycleActiveAndCompatible, makeLifecycleSelfSupport, and makeLifecycleIncompatible so the mock payload stays consistent when the schema or version shape changes.frontend/e2e/tests/olm/operator-lifecycle-metadata.spec.ts (2)
154-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent fallback on version-parse failure hides the real cause of later assertion failures.
If
SERVER_FLAGS.releaseVersionis missing or has an unexpected format,clusterMinorVersionsilently defaults to'4.18'. If that doesn't match the actual cluster version the UI compares against, the "compatible" step assertion will fail with no indication that the real root cause was a version-extraction problem rather than a UI/lifecycle-data bug.♻️ Proposed fix to surface parse failures
const versionMatch = releaseVersion.match(/^(\d+\.\d+)/); - const clusterMinorVersion = versionMatch ? versionMatch[1] : '4.18'; + if (!versionMatch) { + throw new Error(`Unable to parse cluster minor version from releaseVersion: "${releaseVersion}"`); + } + const clusterMinorVersion = versionMatch[1];🤖 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 `@frontend/e2e/tests/olm/operator-lifecycle-metadata.spec.ts` around lines 154 - 159, The version parsing in operator-lifecycle-metadata.spec.ts is silently falling back to a hardcoded minor version, which hides the real failure source. Update the logic around the SERVER_FLAGS.releaseVersion handling so the test explicitly fails or reports a clear error when the releaseVersion is missing or doesn’t match the expected format, instead of defaulting to 4.18. Use the existing releaseVersion, versionMatch, and clusterMinorVersion flow in the Installed Operators test to surface the parse problem before the later compatibility assertion runs.
98-134: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCleanup errors are fully swallowed, risking silent resource leaks across CI runs.
Both catch blocks discard every error without logging. If deletion genuinely fails (e.g., permissions, API changes), the Subscription/CSVs leak into subsequent runs with no diagnostic trail.
♻️ Proposed fix to log instead of silently swallowing
} catch { - // Ignore cleanup errors + // eslint-disable-next-line no-console + console.warn('Failed to delete web-terminal subscription during cleanup'); }🤖 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 `@frontend/e2e/tests/olm/operator-lifecycle-metadata.spec.ts` around lines 98 - 134, The cleanup in test.afterAll currently swallows all errors when deleting the Subscription and CSVs, which can hide leaked resources; update both catch blocks to log the failure with enough context before continuing. Use the existing k8sClient.deleteCustomResource and k8sClient.listCustomResources flow in operator-lifecycle-metadata.spec.ts, and include the resource type/name details in the log so CI failures can be diagnosed if cleanup does not succeed.
🤖 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 `@frontend/e2e/mocks/operator-lifecycle.ts`:
- Around line 1-84: The lifecycle mock shape is duplicated across LifecycleData,
activePhases/expiredPhases, and the three factory helpers, so centralize the
shared phase/platformCompatibility/version object types and the common
schema/versions wrapper. Reuse those shared symbols from operator-lifecycle.ts
inside makeLifecycleActiveAndCompatible, makeLifecycleSelfSupport, and
makeLifecycleIncompatible so the mock payload stays consistent when the schema
or version shape changes.
In `@frontend/e2e/tests/olm/operator-lifecycle-metadata.spec.ts`:
- Around line 154-159: The version parsing in
operator-lifecycle-metadata.spec.ts is silently falling back to a hardcoded
minor version, which hides the real failure source. Update the logic around the
SERVER_FLAGS.releaseVersion handling so the test explicitly fails or reports a
clear error when the releaseVersion is missing or doesn’t match the expected
format, instead of defaulting to 4.18. Use the existing releaseVersion,
versionMatch, and clusterMinorVersion flow in the Installed Operators test to
surface the parse problem before the later compatibility assertion runs.
- Around line 98-134: The cleanup in test.afterAll currently swallows all errors
when deleting the Subscription and CSVs, which can hide leaked resources; update
both catch blocks to log the failure with enough context before continuing. Use
the existing k8sClient.deleteCustomResource and k8sClient.listCustomResources
flow in operator-lifecycle-metadata.spec.ts, and include the resource type/name
details in the log so CI failures can be diagnosed if cleanup does not succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 56b1ada9-9c89-47fd-ac4f-ddc8c32c8093
📒 Files selected for processing (2)
frontend/e2e/mocks/operator-lifecycle.tsfrontend/e2e/tests/olm/operator-lifecycle-metadata.spec.ts
c2bc5f0 to
d651a59
Compare
|
/retest |
1 similar comment
|
/retest |
d651a59 to
122fdf3
Compare
perdasilva
changed the title
|
@perdasilva: This pull request references CONSOLE-5409 which is a valid 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. |
122fdf3 to
f1e021d
Compare
|
@fsgreco thanks for the review!! I've put the suggested changes through ^^ |
b121bf5 to
0778655
Compare
|
Scheduling tests matching the |
|
/verified by "ci/prow/e2e-playwright" |
openshift-ci-robot
added
the
verified
|
@fsgreco: 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. |
|
/hold until github.com/openshift/release/pull/81559 is merged as this will likely need an update and it would be good to get the techpreview job signal before merging this |
openshift-ci
Bot
added
the
do-not-merge/hold
|
as far as i can tell, nothing user facing here! |
|
/label docs-approved |
|
/test backend |
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
TheRealJon
left a comment
TheRealJon
left a comment
There was a problem hiding this comment.
I have a few comments, but since these tests are passing, let's get them merged and maybe open a follow-on PR to make improvements.
/lgtm
|
Scheduling tests matching the |
openshift-ci
Bot
added
the
approved
|
/retest |
Adds e2e test coverage for the operator lifecycle metadata columns (Cluster Compatibility and Support Phase) on the Installed Operators page. Test files: - operator-lifecycle-metadata.spec.ts: Installs web-terminal operator, mocks lifecycle API responses, and verifies three states (compatible/ active support, unsupported, incompatible) - operator-lifecycle.ts: Mock data helpers with dynamic dates - installed-operators-page.ts: Page object for Installed Operators Also adds: - data-test attribute on ClusterServiceVersionTableRow for getByTestId() - test-prow-playwright-e2e-techpreview.sh as CI entrypoint Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c8db489 to
fc362e5
Compare
|
/test e2e-gcp-console-techpreview |
1 similar comment
|
/test e2e-gcp-console-techpreview |
|
/retest |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fsgreco, perdasilva, TheRealJon 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 |
|
/retest |
5 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/test e2e-playwright |
1 similar comment
|
/test e2e-playwright |
|
Probably need #16780 to land before |
|
/retest |
|
@perdasilva: The following test failed, say
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. |
7 participants
Analysis / Root cause:
The operator lifecycle metadata UI (Cluster Compatibility and Support Phase columns on the Installed Operators page) had no e2e test coverage. Since no catalog with real lifecycle metadata exists yet, the test uses mocked API responses to verify the UI renders correctly across all three states.
Solution description:
Adds a Playwright e2e test (
frontend/e2e/tests/olm/operator-lifecycle-metadata.spec.ts), mock data helpers (frontend/e2e/mocks/operator-lifecycle.ts), and anInstalledOperatorsPagepage object (frontend/e2e/pages/installed-operators-page.ts) that:web-terminaloperator from theredhat-operatorscatalog viak8sClientsubscription.status.installedCSVto detect when the operator CSV reachesSucceededphase/api/olm/lifecycle/**endpoint usingpage.route()with a mutable reference pattern (consistent withupdate-modal.spec.ts)releaseVersionfromSERVER_FLAGSto build accurate mock dataOLMLifecycleAndCompatibilityfeature gate enabledtest.stepblocks:afterAllusingPromise.allSettledso failures in one deletion don't block the othersThe page object encapsulates locator composition — indicator/badge methods take
displayNamedirectly and resolve the operator row internally, keeping specs free of intermediate locators.navigateTo()accepts an optional namespace parameter, falling back to the all-namespaces view when omitted.Also adds a
data-testattribute to the operator row link inClusterServiceVersionTableRowalongside the existing legacydata-test-operator-row, enabling idiomaticgetByTestId()usage in the page object.Mock dates are computed dynamically relative to the current date to avoid time-bomb failures.
Implements
test-prow-playwright-e2e-techpreview.shas the Prow CI entrypoint for the test: reads cluster credentials, callscontrib/create-user.sh, then runs theolmPlaywright project scoped tooperator-lifecycle-metadata.spec.ts.Screenshots / screen recording:
Test setup:
OLMLifecycleAndCompatibilityfeature gate enabled (for theOPERATOR_LIFECYCLE_METADATAfeature flag)redhat-operatorsCatalogSource must be available inopenshift-marketplacetest-prow-playwright-e2e-techpreview.shTest cases:
Browser conformance:
Additional info:
Verified against a live OCP 5.0 nightly cluster with the OLMLifecycleAndCompatibility feature gate enabled.
🤖 Generated with Claude Code