CONSOLE-5417: Migrate dev-console Cypress tests to Playwright (batch 4)#16767
CONSOLE-5417: Migrate dev-console Cypress tests to Playwright (batch 4)#16767shahsahil264 wants to merge 10 commits into
Conversation
Migrate vulnerability spec (4 tests) with VulnerabilityPage page object. Delete 13 Cypress features that were @broken-test/@to-do/@Manual (CRW, operators, getting-started-tour, project-access, service-mesh, virtualization) — no Playwright equivalents needed. Add data-test attrs to filter-toolbar.tsx for filter-dropdown-toggle and row-filter checkboxes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The quarkus test image requires privileged PodSecurity to run. Also adds exact:true to Create button to avoid strict mode violation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move deployment creation to beforeAll with waitForIMVData polling
- Add warmupSPA + switchPerspective('Developer') in beforeEach
- Replace [id] and [aria-label] selectors with getByRole where possible
- Adapt TC05 to use vulnerability type toggle buttons (OCP 5.0 UI)
- Remove 8 unused methods and 2 dead locator properties
- Add sleep infinity + restricted securityContext for quarkus on OCP 5.0
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@shahsahil264: This pull request references CONSOLE-5417 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 sub-task to target the "5.0.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. |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shahsahil264 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 |
|
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:
WalkthroughAdded Playwright end-to-end coverage for Dev Console image vulnerabilities, including Kubernetes fixtures, operator detection, page-object helpers, filtering, and detail validation. Updated test selectors and removed selected legacy integration coverage. ChangesVulnerability E2E coverage
Legacy integration test cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant vulnerability.spec.ts
participant KubernetesClient
participant DevConsole
participant VulnerabilityPage
vulnerability.spec.ts->>KubernetesClient: create Deployment and poll ImageManifestVuln resources
KubernetesClient-->>vulnerability.spec.ts: return vulnerability data
vulnerability.spec.ts->>DevConsole: warm SPA and switch to Developer perspective
vulnerability.spec.ts->>VulnerabilityPage: navigate and open vulnerability views
VulnerabilityPage->>DevConsole: interact with tabs, filters, alerts, images, and type buttons
DevConsole-->>vulnerability.spec.ts: render vulnerability tables and details
Suggested reviewers: 🚥 Pre-merge checks | ✅ 12 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
# Conflicts: # frontend/e2e/pages/base-page.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
frontend/e2e/pages/dev-console/vulnerability-page.ts (1)
75-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate locator construction between getters and click helpers.
clickVulnLink,clickVulnImageLink, andclickVulnerabilityTypeeach re-build the same locator that their correspondinggetVulnLink,getVulnImageLink, andgetVulnerabilityTypeButtonalready expose, instead of delegating to them. If a locator changes, both copies must be updated in sync.♻️ Proposed refactor to reuse the getters
async clickVulnLink(index: number): Promise<void> { - await this.robustClick(this.page.getByTestId(`vuln-${index}`)); + await this.robustClick(this.getVulnLink(index)); } getVulnImageLink(imageName: string): Locator { return this.page.getByRole('link', { name: imageName }); } async clickVulnImageLink(imageName: string): Promise<void> { - await this.robustClick(this.page.getByRole('link', { name: imageName })); + await this.robustClick(this.getVulnImageLink(imageName)); } ... async clickVulnerabilityType(typeName: string): Promise<void> { - await this.robustClick(this.page.getByRole('button', { name: typeName, exact: true })); + await this.robustClick(this.getVulnerabilityTypeButton(typeName)); }🤖 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/pages/dev-console/vulnerability-page.ts` around lines 75 - 101, Update clickVulnLink, clickVulnImageLink, and clickVulnerabilityType to pass their corresponding getVulnLink, getVulnImageLink, and getVulnerabilityTypeButton results to robustClick instead of rebuilding locators. Leave the getter locator definitions unchanged.
🤖 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 `@frontend/e2e/tests/dev-console/fixtures/create-quarkus-deployment.yaml`:
- Around line 15-30: Update the quarkus container definition in
create-quarkus-deployment.yaml to include CPU and memory resource limits, plus
liveness and readiness probes compatible with the sleep-based container. Use
bounded fixture-appropriate values and probes that reliably report the pod as
healthy and ready, while preserving the existing security settings.
- Around line 15-30: Harden the create-quarkus-deployment fixture by setting the
container securityContext to readOnlyRootFilesystem: true and disabling
service-account token mounting at the pod spec level with
automountServiceAccountToken: false. Keep the existing sleep command and other
security settings unchanged.
In `@frontend/e2e/tests/dev-console/utils/operator-check.ts`:
- Around line 12-28: Update hasOperatorSubscription to return false only when
getCustomResource reports a not-found error for the subscription; rethrow
authentication, RBAC, network, API-server, malformed-response, and all other
unexpected errors so setup fails instead of skipping the suite.
In `@frontend/e2e/tests/dev-console/vulnerability.spec.ts`:
- Around line 15-27: Update waitForIMVData to catch errors from
listCustomResources within each polling iteration and continue retrying until
timeout, preserving the existing success condition and 10-second delay. Only
throw the final timeout error after the full 180-second window expires, rather
than propagating transient API or CRD-registration failures immediately.
- Around line 41-43: Update the namespace options in the test setup around
k8sClient.createNamespace to use the least-permissive pod security enforcement
level, preferably restricted or baseline, since the fixture pod already
complies. Retain privileged only if another fixture requires it, and document
that requirement with a brief note.
---
Nitpick comments:
In `@frontend/e2e/pages/dev-console/vulnerability-page.ts`:
- Around line 75-101: Update clickVulnLink, clickVulnImageLink, and
clickVulnerabilityType to pass their corresponding getVulnLink,
getVulnImageLink, and getVulnerabilityTypeButton results to robustClick instead
of rebuilding locators. Leave the getter locator definitions unchanged.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 935e9232-80c0-47ae-8df8-b613c0c31bf3
📒 Files selected for processing (25)
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/dev-console/vulnerability-page.tsfrontend/e2e/tests/dev-console/fixtures/create-quarkus-deployment.yamlfrontend/e2e/tests/dev-console/utils/operator-check.tsfrontend/e2e/tests/dev-console/vulnerability.spec.tsfrontend/packages/dev-console/integration-tests/features/codeready-workspaces/crw-basic.featurefrontend/packages/dev-console/integration-tests/features/codeready-workspaces/open-app-in-crw.featurefrontend/packages/dev-console/integration-tests/features/common/operators.featurefrontend/packages/dev-console/integration-tests/features/getting-started/getting-started-to-dev-perspective.featurefrontend/packages/dev-console/integration-tests/features/getting-started/getting-started-tour-dev-perspective.featurefrontend/packages/dev-console/integration-tests/features/project/project-access.featurefrontend/packages/dev-console/integration-tests/features/service-mesh/service-mesh-console-links-admin.featurefrontend/packages/dev-console/integration-tests/features/service-mesh/service-mesh-console-links-nonadmin.featurefrontend/packages/dev-console/integration-tests/features/virtulization/actions-on-vm.featurefrontend/packages/dev-console/integration-tests/features/virtulization/create-vm-from-template.featurefrontend/packages/dev-console/integration-tests/features/virtulization/virtual-machines-page.featurefrontend/packages/dev-console/integration-tests/features/virtulization/virtual-machines-template.featurefrontend/packages/dev-console/integration-tests/features/vulnerability/project-vulnerability.featurefrontend/packages/dev-console/integration-tests/support/pageObjects/vulnerability-po.tsfrontend/packages/dev-console/integration-tests/support/pages/crw/authorizeAccess.tsfrontend/packages/dev-console/integration-tests/support/pages/crw/keycloak.tsfrontend/packages/dev-console/integration-tests/support/step-definitions/crw/crw-basic.tsfrontend/packages/dev-console/integration-tests/support/step-definitions/getting-started/getting-started-tour-dev-perspective.tsfrontend/packages/dev-console/integration-tests/support/step-definitions/vulnerability/project-vulnerability.tsfrontend/public/components/filter-toolbar.tsx
💤 Files with no reviewable changes (19)
- frontend/packages/dev-console/integration-tests/support/pageObjects/vulnerability-po.ts
- frontend/packages/dev-console/integration-tests/support/step-definitions/crw/crw-basic.ts
- frontend/packages/dev-console/integration-tests/features/virtulization/virtual-machines-page.feature
- frontend/packages/dev-console/integration-tests/support/pages/crw/keycloak.ts
- frontend/packages/dev-console/integration-tests/features/virtulization/create-vm-from-template.feature
- frontend/packages/dev-console/integration-tests/features/getting-started/getting-started-to-dev-perspective.feature
- frontend/packages/dev-console/integration-tests/features/codeready-workspaces/open-app-in-crw.feature
- frontend/packages/dev-console/integration-tests/features/project/project-access.feature
- frontend/packages/dev-console/integration-tests/features/virtulization/virtual-machines-template.feature
- frontend/packages/dev-console/integration-tests/features/vulnerability/project-vulnerability.feature
- frontend/packages/dev-console/integration-tests/features/codeready-workspaces/crw-basic.feature
- frontend/packages/dev-console/integration-tests/features/service-mesh/service-mesh-console-links-nonadmin.feature
- frontend/packages/dev-console/integration-tests/support/step-definitions/getting-started/getting-started-tour-dev-perspective.ts
- frontend/packages/dev-console/integration-tests/features/common/operators.feature
- frontend/packages/dev-console/integration-tests/features/getting-started/getting-started-tour-dev-perspective.feature
- frontend/packages/dev-console/integration-tests/support/pages/crw/authorizeAccess.ts
- frontend/packages/dev-console/integration-tests/features/service-mesh/service-mesh-console-links-admin.feature
- frontend/packages/dev-console/integration-tests/features/virtulization/actions-on-vm.feature
- frontend/packages/dev-console/integration-tests/support/step-definitions/vulnerability/project-vulnerability.ts
|
/test e2e-gcp-console |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
- Add resource limits, probes, readOnlyRootFilesystem to quarkus fixture - Set automountServiceAccountToken: false on pod spec - Use restricted PSA level instead of privileged (container complies) - Rethrow non-404 errors in hasOperatorSubscription - Catch transient API errors in waitForIMVData polling loop - Reuse getter methods in click helpers to avoid locator duplication Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/test e2e-gcp-console |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/e2e/tests/dev-console/utils/operator-check.ts (1)
35-59:⚠️ Potential issue | 🟠 MajorDo not treat every CSV-list failure as “not ready.”
Lines 56–58 convert RBAC, authentication, network, API-server, and malformed-response errors into
false. This can make a broken cluster look like an unavailable operator or cause polling to time out without the real cause. Returnfalseonly for the expected not-found case and rethrow other errors, or handle transient retries in the caller.🤖 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/dev-console/utils/operator-check.ts` around lines 35 - 59, Update hasCSVReady so it returns false only when listCustomResources reports the expected not-found condition; rethrow authentication, authorization, network, API-server, and malformed-response errors instead of swallowing them. Preserve the existing CSV matching and readiness behavior for successful responses.
🤖 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.
Outside diff comments:
In `@frontend/e2e/tests/dev-console/utils/operator-check.ts`:
- Around line 35-59: Update hasCSVReady so it returns false only when
listCustomResources reports the expected not-found condition; rethrow
authentication, authorization, network, API-server, and malformed-response
errors instead of swallowing them. Preserve the existing CSV matching and
readiness behavior for successful responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aaa23f1e-81c7-47be-ad92-39232bd8e836
📒 Files selected for processing (5)
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/dev-console/vulnerability-page.tsfrontend/e2e/tests/dev-console/fixtures/create-quarkus-deployment.yamlfrontend/e2e/tests/dev-console/utils/operator-check.tsfrontend/e2e/tests/dev-console/vulnerability.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/e2e/tests/dev-console/fixtures/create-quarkus-deployment.yaml
- frontend/e2e/pages/dev-console/vulnerability-page.ts
- frontend/e2e/tests/dev-console/vulnerability.spec.ts
…Subscription - Restore vulnerability-po.ts deleted by migration — still imported by Cypress barrel export in pageObjects/index.ts - Broaden 404 detection to handle @kubernetes/client-node error format where statusCode may be in .code or error message string Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 `@frontend/e2e/tests/dev-console/utils/operator-check.ts`:
- Around line 22-31: The not-found detection in the operator-check error
handling is too broad because msg.includes('404') can classify unrelated errors
as missing subscriptions. Update the fallback after the structured code check to
use only a bounded, explicit not-found message pattern, or remove the message
fallback entirely, while preserving the existing structured status-field
handling and false return for genuine 404 responses.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 85e86d98-6dfa-4afc-b72e-7b92f2e4bf11
📒 Files selected for processing (1)
frontend/e2e/tests/dev-console/utils/operator-check.ts
Structured .code check already covers ApiException from @kubernetes/client-node. Message matching was too broad. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/test e2e-gcp-console |
|
/retest |
1 similar comment
|
/retest |
|
@shahsahil264: 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. |
- Delete unused create-quarkus-deployment.yaml fixture (dead code, hardcoded namespace, inconsistent probes vs inline spec) - Move operator-check.ts to e2e/utils/ for cross-suite reuse - Add severity filter assertions to TC05 for feature parity with original Cypress test - Add getFilterOption() to VulnerabilityPage for locator reuse Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/test e2e-gcp-console |
Analysis / Root cause:
Part of the Cypress → Playwright migration epic (CONSOLE-5196). This is batch 4 of the migration, covering the image vulnerability tests from the dev-console package.
Jira: CONSOLE-5417
Solution description:
Migrates the
project-vulnerability.featureCypress tests to Playwright, covering 4 test scenarios for the Quay Container Security Operator's Image Manifest Vulnerabilities UI:PV-01-TC02 was
@manualin Cypress and is not migrated.Key implementation details:
beforeAllchecks for the subscription and skips the entire suite if not installedbeforeAllfor efficiency (Strategy B isolation)waitForIMVData()polls the K8s API forImageManifestVulnresources before any UI interactionwarmupSPA+switchPerspective('Developer')inbeforeEach— Vulnerabilities tab only appears in Developer perspectivesleep infinitycommand override and restricted securityContext on OCP 5.0pod-security.kubernetes.io/enforce=restrictedlabel (pod meets restricted profile)Files added:
e2e/pages/dev-console/vulnerability-page.ts— page object with locators and actionse2e/tests/dev-console/vulnerability.spec.ts— test spec with 4 scenariose2e/utils/operator-check.ts— operator subscription check utility (shared across suites)Files deleted:
@manual/@broken-teste2e/tests/dev-console/fixtures/create-quarkus-deployment.yaml— removed dead code (deployment created inline)Files modified:
e2e/pages/base-page.ts— importexpectfrom fixtures, usegetByTestIdfor perspective switcherpublic/components/filter-toolbar.tsx— addeddata-testattribute forgetByTestIdcompatibilityScreenshots / screen recording:
Test setup:
Test cases:
Browser conformance:
Additional info:
/migrate-cypressskillReviewers and assignees:
/cc @rhamilto