NO-ISSUE: Add Playwright e2e test for operator lifecycle metadata UI#16701
NO-ISSUE: Add Playwright e2e test for operator lifecycle metadata UI#16701perdasilva wants to merge 1 commit into
Conversation
|
@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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: perdasilva 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds lifecycle mock factories and a Playwright e2e spec that provisions an OLM subscription, intercepts lifecycle API responses, and verifies lifecycle metadata labels for compatible, self-support, and incompatible states. ChangesOperator Lifecycle E2E Test
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant K8sAPI
participant Browser
participant LifecycleAPI
Test->>K8sAPI: create Subscription
K8sAPI-->>Test: CSV reaches Succeeded
Test->>LifecycleAPI: register mocked lifecycle route
Test->>Browser: open installed operators page
Browser->>LifecycleAPI: fetch lifecycle data
LifecycleAPI-->>Browser: mocked payload
Browser-->>Test: render lifecycle labels
Test->>Browser: reload with updated mock
Browser-->>Test: assert updated lifecycle label
Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
Adds an e2e test that verifies the lifecycle metadata columns (Cluster Compatibility and Support Phase) on the Installed Operators page using mocked lifecycle API responses. The test is automatically skipped on clusters without tech preview enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c2bc5f0 to
d651a59
Compare
|
/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. |
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) and mock data helpers (frontend/e2e/mocks/operator-lifecycle.ts) that:web-terminaloperator from theredhat-operatorscatalog viak8sClient/api/olm/lifecycle/**endpoint usingpage.route()with a mutable reference pattern (consistent withupdate-modal.spec.ts)releaseVersionfromSERVER_FLAGSto build accurate mock datatest.stepblocks:afterAllMock dates are computed dynamically relative to the current date to avoid time-bomb failures.
Screenshots / screen recording:
Test setup:
TechPreviewNoUpgradefeature gate enabled (for theOPERATOR_LIFECYCLE_METADATAfeature flag)redhat-operatorsCatalogSource must be available inopenshift-marketplaceTest cases:
Browser conformance:
Additional info:
Verified against a live OCP 5.0 nightly cluster with tech preview enabled.
🤖 Generated with Claude Code
Summary by CodeRabbit