Skip to content

CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright#16544

Open
rhamilto wants to merge 12 commits into
openshift:mainfrom
rhamilto:CONSOLE-5278
Open

CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright#16544
rhamilto wants to merge 12 commits into
openshift:mainfrom
rhamilto:CONSOLE-5278

Conversation

@rhamilto

@rhamilto rhamilto commented Jun 3, 2026

Copy link
Copy Markdown
Member

Analysis / Root cause:
Migrating Cypress e2e tests to Playwright as part of the CONSOLE-5196 Playwright migration epic. This story covers 5 Cypress test files under packages/integration-tests/tests/crud/.

Solution description:
Migrates all 5 CRUD Cypress test files to Playwright:

  • other-routes.cy.tse2e/tests/console/crud/other-routes.spec.ts — verifies 20 routes load without errors (dashboard, API explorer, cluster settings, nodes, machines, etc.) plus perspective query parameter switching. Each route includes an axe-core a11y audit.
  • quotas.cy.tse2e/tests/console/crud/quotas.spec.ts — creates ResourceQuota and ClusterResourceQuota via YAML editor, verifies list filtering across All Projects and project-scoped views
  • roles-rolebindings.cy.tse2e/tests/console/crud/roles-rolebindings.spec.ts — creates Roles/ClusterRoles via YAML editor and RoleBindings/ClusterRoleBindings via form, verifies rules table columns and breadcrumb project dropdown restoration
  • image-pull-secret.cy.tse2e/tests/console/crud/image-pull-secret.spec.ts — creates image pull secret via form, verifies whitespace trimming
  • add-storage-crud.cy.tse2e/tests/console/crud/add-storage-crud.spec.ts — creates each workload type via YAML editor, adds storage, verifies volume table

Infrastructure additions:

  • Shared Monaco editor helpers (getEditorContent/setEditorContent) extracted to base-page.ts with waitForFunction guards for reliable editor readiness
  • Cluster-scoped create/delete methods added to KubernetesClient (including deleteClusterCustomResource which swallows 404s)
  • createConfigMap, createSecret, mergePatchResource, annotateConfigMap, labelConfigMap methods added to KubernetesClient (shared with PR [WIP] CONSOLE-5277: Migrate console e2e CRUD tests to Playwright #16556)
  • Cluster resource tracking added to the cleanup fixture
  • data-test attributes added alongside data-test-id on Breadcrumbs, actions menu button, kebab action items, action menu items, perspective switcher, volumes table, and other UI elements
  • New page objects: RoleBindingPage, extended ListPage (filterByCheckbox, legacy locators/methods, kebab actions, showSystemSwitch toggle), extended DetailsPage (getBreadcrumb, clickPageAction, clickKebabAction)
  • Accessibility testing utility (a11y.ts) using @axe-core/playwright
  • Removed all DetailsPage.waitForPageLoad() callers — Playwright actions and assertions auto-wait, making the explicit page-load wait redundant
  • Updated migration docs to include deleteClusterCustomResource in the no-try/catch cleanup guidance

Stacked on PR #16583 (playwright-waitfor-cleanup) which removes redundant waitFor calls and DetailsPage.waitForPageLoad() from the shared page objects.

One test is marked test.fixme: "RoleBindings breadcrumb does not restore All Projects dropdown" — the Cypress version only passed due to testIsolation:false carrying state from prior tests.

Screenshots / screen recording:

Test setup:

  • Running OpenShift cluster with oc login
  • Console running locally on http://localhost:9000
  • frontend/e2e/.env configured with cluster credentials

Test cases:

  • npx playwright test --project=console e2e/tests/console/crud/ --retries=0 — 50 tests pass, 1 skipped (fixme), 1 pre-existing a11y failure on Machine list page (PatternFly MenuToggle missing accessible name)

Browser conformance:

  • Chrome

Additional info:

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 3, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

@rhamilto: This pull request references CONSOLE-5278 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 story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Analysis / Root cause:
Migrating Cypress e2e tests to Playwright as part of the CONSOLE-5196 Playwright migration epic. This story covers 5 Cypress test files (15 tests) under packages/integration-tests/tests/crud/.

Solution description:
Migrates all 5 CRUD Cypress test files to Playwright:

  • other-routes.cy.ts (3 tests) → e2e/tests/console/crud/other-routes.spec.ts — verifies error page, API explorer, and cluster settings routes
  • quotas.cy.ts (5 tests) → e2e/tests/console/crud/quotas.spec.ts — creates ResourceQuota and ClusterResourceQuota via YAML editor, verifies list filtering across projects
  • roles-rolebindings.cy.ts (10 tests) → e2e/tests/console/crud/roles-rolebindings.spec.ts — creates Roles and RoleBindings via YAML editor and form, verifies rules table columns and breadcrumb project dropdown restoration
  • image-pull-secret.cy.ts (1 test) → e2e/tests/console/crud/image-pull-secret.spec.ts — creates image pull secret via form, verifies whitespace trimming
  • add-storage-crud.cy.ts (6 tests) → e2e/tests/console/crud/add-storage-crud.spec.ts — creates each workload type via YAML editor, adds storage, verifies volume table

Infrastructure additions:

  • Shared Monaco editor helpers (getEditorContent/setEditorContent) extracted to base-page.ts
  • Cluster-scoped create/delete methods added to KubernetesClient
  • Cluster resource tracking added to the cleanup fixture
  • data-test attributes added alongside data-test-id on Breadcrumbs, actions menu button, and kebab action items
  • New page objects: RoleBindingPage, extended ListPage (filterByCheckbox), extended DetailsPage (getBreadcrumb, clickPageAction)

One test is marked test.fixme: "RoleBindings breadcrumb does not restore All Projects dropdown" — the Cypress version only passed due to testIsolation:false carrying state from prior tests.

Screenshots / screen recording:

Test setup:

  • Running OpenShift cluster with oc login
  • Console running locally on http://localhost:9000
  • frontend/e2e/.env configured with cluster credentials

Test cases:

  • npx playwright test --project=console e2e/tests/console/crud/ --retries=0 — all 25 tests pass (1 skipped fixme)

Browser conformance:

  • Chrome

Additional info:

  • Jira: CONSOLE-5278
  • Parent epic: CONSOLE-5196
  • All original Cypress files and orphaned view files deleted after validation

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

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR updates Cypress→Playwright migration docs/selectors, adds Monaco editor helpers, cluster-CR support, page objects, accessibility tooling, many Playwright test suites, data-test attributes across the UI, and removes corresponding Cypress specs.

Changes

Cypress→Playwright E2E migration with infrastructure and test coverage expansion

Layer / File(s) Summary
Migration documentation and selector strategy
.claude/migration-context.md, .claude/skills/migrate-cypress/SKILL.md
Updated migration guidance to mandate page.getByTestId() and instruct adding data-test attributes to React components (preserving legacy attributes); page-object examples updated.
Monaco editor helpers (BasePage)
frontend/e2e/pages/base-page.ts, frontend/e2e/pages/alertmanager-page.ts, frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts
Added exported getEditorContent/setEditorContent helpers and BasePage wrappers; refactored alertmanager and CRD test utils to use them.
Kubernetes client & cleanup fixture
frontend/e2e/clients/kubernetes-client.ts, frontend/e2e/fixtures/cleanup-fixture.ts
Added cluster-scoped custom resource create/delete methods to KubernetesClient and trackClusterCustomResource plus cleanup branch for cluster-scoped resources.
Playwright page-object library
frontend/e2e/pages/details-page.ts, frontend/e2e/pages/list-page.ts, frontend/e2e/pages/role-binding-page.ts
Introduced DetailsPage.clickPageAction/getBreadcrumb, ListPage page-object (filters, cell access, selectProject), and RoleBindingPage for RBAC form flows.
Playwright test suites (new)
frontend/e2e/tests/console/crud/*
Added Playwright specs: add-storage-crud, image-pull-secret, other-routes, quotas, roles-rolebindings with YAML and form-driven flows and namespace lifecycle.
Accessibility testing utility and dependency
frontend/e2e/utils/a11y.ts, frontend/package.json
Added testA11y() integrating axe-core with Playwright and added @axe-core/playwright devDependency.
Component data-test attributes
frontend/packages/console-app/src/components/nav/NavHeader.tsx, frontend/packages/console-shared/src/components/breadcrumbs/Breadcrumbs.tsx, frontend/public/components/*, frontend/packages/console-shared/src/components/actions/menu/*
Added data-test attributes across UI components (nav/perspective, breadcrumbs, API explorer, cluster settings, error page, table rows, actions menu, kebab items, action menu items) to support getByTestId() selector strategy.
Cypress test removal (migration)
frontend/packages/integration-tests/tests/crud/*.cy.ts, frontend/packages/integration-tests/views/rolebindings.ts
Removed corresponding Cypress specs and helper view (add-storage-crud.cy.ts, image-pull-secret.cy.ts, other-routes.cy.ts, roles-rolebindings.cy.ts, and rolebindings.ts) as functionality moved to Playwright.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

ok-to-test

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PR contains only Playwright tests (TypeScript/.spec.ts), not Ginkgo tests. Ginkgo check is not applicable to this frontend E2E test migration PR.
Test Structure And Quality ✅ Passed Custom check asks for Ginkgo test code review, but PR contains only Playwright (TypeScript) tests. No Ginkgo code exists in the PR or e2e directory. Check is not applicable.
Microshift Test Compatibility ✅ Passed PR adds Playwright e2e tests (TypeScript), not Ginkgo tests (Go). Custom check is specific to Ginkgo e2e tests, so it is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR adds Playwright e2e tests for the console UI (TypeScript .spec.ts), not Ginkgo infrastructure tests (Go). The check only applies to Ginkgo tests with It()/Describe() syntax.
Topology-Aware Scheduling Compatibility ✅ Passed PR contains E2E test migration and frontend test attributes only; no deployment manifests, operator code, controllers, or scheduling constraints are introduced.
Ote Binary Stdout Contract ✅ Passed This PR modifies only frontend code (TypeScript/JavaScript/React components, Playwright tests). The OTE Binary Stdout Contract check applies to Go test binaries; no Go code is modified in this PR.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR adds Playwright tests (TypeScript), not Ginkgo e2e tests (Go). The custom check applies only to Ginkgo tests; therefore it is not applicable to this PR.
No-Weak-Crypto ✅ Passed No weak cryptography found in PR-modified files: no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, no custom crypto, no insecure secret comparisons. OAuth token handling uses safe HTTPS+base64.
Container-Privileges ✅ Passed PR contains only test files, React UI components, and test infrastructure—no Kubernetes manifests, Dockerfiles, or container configurations that could have privileged settings.
No-Sensitive-Data-In-Logs ✅ Passed No logging of sensitive data detected. All new test code is logging-free; existing cleanup fixture logging only captures generic error messages and resource names without credentials or PII.
Title check ✅ Passed The title 'CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright' accurately and concisely describes the main objective of the changeset, which is to migrate five CRUD test suites from Cypress to Playwright while adding supporting infrastructure.
Description check ✅ Passed The pull request description is well-structured and comprehensive, covering all required template sections with detailed information about the migration scope, infrastructure changes, and test coverage.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@rhamilto rhamilto changed the title CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright [WIP] CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright Jun 3, 2026
@openshift-ci openshift-ci Bot requested review from cajieh and jhadvig June 3, 2026 21:21
@openshift-ci openshift-ci Bot added component/core Related to console core functionality component/shared Related to console-shared approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cypress Related to Cypress e2e integration testing labels Jun 3, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
frontend/e2e/tests/console/crud/quotas.spec.ts (2)

22-38: 💤 Low value

Consider the deleteClusterCustomResource wrapper for consistency.

The roles spec deletes cluster-scoped resources via k8sClient.deleteClusterCustomResource(...), while here you call k8sClient.customObjectsApi.deleteClusterCustomObject(...) directly. Using the shared wrapper keeps cleanup uniform across the migrated suites.

🤖 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/console/crud/quotas.spec.ts` around lines 22 - 38, Replace
the direct call to k8sClient.customObjectsApi.deleteClusterCustomObject(...)
with the shared wrapper k8sClient.deleteClusterCustomResource(...) to keep
cleanup consistent with other suites; in the test.afterAll block, call
deleteClusterCustomResource with the same group 'quota.openshift.io', version
'v1', plural 'clusterresourcequotas', and name clusterQuotaName (and keep the
existing try/catch), and retain the existing
k8sClient.deleteNamespace(namespace) cleanup for namespace removal.

15-20: ⚡ Quick win

Remove dead browser.newPage()/close() code.

These two lines open and immediately close a blank page, accomplishing nothing, and pull an unused browser fixture into beforeAll. Looks like a migration leftover.

🧹 Proposed cleanup
-  test.beforeAll(async ({ browser, k8sClient }) => {
+  test.beforeAll(async ({ k8sClient }) => {
     namespace = `test-quotas-${Date.now()}`;
     await k8sClient.createNamespace(namespace);
-    const page = await browser.newPage();
-    await page.close();
   });
🤖 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/console/crud/quotas.spec.ts` around lines 15 - 20, Remove
the dead browser usage in the test.beforeAll hook: delete the calls to
browser.newPage() and page.close() and remove the browser parameter from the
beforeAll callback so the hook only takes ({ k8sClient }) (leaving namespace
creation via k8sClient intact). This eliminates the no-op page open/close and
stops pulling the unused browser fixture into test.beforeAll.
🤖 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/pages/role-binding-page.ts`:
- Around line 26-30: The selectRole method currently ignores its role parameter
by always clicking the hardcoded '`#cluster-admin-ClusterRole-link`'; update
selectRole to build the dropdown option locator from the provided role (e.g.,
`${role}-ClusterRole-link`) and click that locator instead of the fixed string.
Make sure to CSS-escape or otherwise sanitize the role string (dot/other special
chars) before embedding into the selector (or use a safer locator strategy like
locating by attribute or exact text) so roles with characters like '.' are
matched correctly; update references in the selectRole function to use the
constructed/sanitized locator rather than '`#cluster-admin-ClusterRole-link`'.

In `@frontend/e2e/tests/console/crud/add-storage-crud.spec.ts`:
- Line 78: The submit button locator
page.locator('button[type="submit"]').click() is too generic and can match
multiple forms; scope it to the Add-storage modal/form by targeting the modal
container (e.g., the Add-storage modal selector or form element) before
selecting its submit button or switch to a dedicated stable data-test id for the
Add-storage submit (e.g., data-test-add-storage-submit); update the locator
usage in the test so it locates the submit button within the Add-storage
container (or uses the new data-test attribute) and then performs the click.

In `@frontend/e2e/tests/console/crud/image-pull-secret.spec.ts`:
- Around line 34-42: The test is race-prone because it uses imageOption.count()
which doesn't auto-wait; replace the conditional check and directly await the
locator click: use the locator returned by page.getByRole('menuitem', { name:
'Image pull secret' }) (the imageOption variable) and call await
imageOption.click() after page.getByTestId('item-create').click(); keep the
following heading assertion (page.getByRole('heading', { level: 1 })) unchanged.

---

Nitpick comments:
In `@frontend/e2e/tests/console/crud/quotas.spec.ts`:
- Around line 22-38: Replace the direct call to
k8sClient.customObjectsApi.deleteClusterCustomObject(...) with the shared
wrapper k8sClient.deleteClusterCustomResource(...) to keep cleanup consistent
with other suites; in the test.afterAll block, call deleteClusterCustomResource
with the same group 'quota.openshift.io', version 'v1', plural
'clusterresourcequotas', and name clusterQuotaName (and keep the existing
try/catch), and retain the existing k8sClient.deleteNamespace(namespace) cleanup
for namespace removal.
- Around line 15-20: Remove the dead browser usage in the test.beforeAll hook:
delete the calls to browser.newPage() and page.close() and remove the browser
parameter from the beforeAll callback so the hook only takes ({ k8sClient })
(leaving namespace creation via k8sClient intact). This eliminates the no-op
page open/close and stops pulling the unused browser fixture into
test.beforeAll.
🪄 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: b990dd8d-dd13-40ad-86d7-982bfab264e5

📥 Commits

Reviewing files that changed from the base of the PR and between 76aa7c8 and 0ea61dc.

⛔ Files ignored due to path filters (1)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (31)
  • .claude/migration-context.md
  • .claude/skills/migrate-cypress/SKILL.md
  • frontend/e2e/clients/kubernetes-client.ts
  • frontend/e2e/fixtures/cleanup-fixture.ts
  • frontend/e2e/pages/alertmanager-page.ts
  • frontend/e2e/pages/base-page.ts
  • frontend/e2e/pages/details-page.ts
  • frontend/e2e/pages/list-page.ts
  • frontend/e2e/pages/role-binding-page.ts
  • frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts
  • frontend/e2e/tests/console/crud/add-storage-crud.spec.ts
  • frontend/e2e/tests/console/crud/image-pull-secret.spec.ts
  • frontend/e2e/tests/console/crud/other-routes.spec.ts
  • frontend/e2e/tests/console/crud/quotas.spec.ts
  • frontend/e2e/tests/console/crud/roles-rolebindings.spec.ts
  • frontend/e2e/utils/a11y.ts
  • frontend/package.json
  • frontend/packages/console-app/src/components/nav/NavHeader.tsx
  • frontend/packages/console-shared/src/components/breadcrumbs/Breadcrumbs.tsx
  • frontend/packages/integration-tests/tests/crud/add-storage-crud.cy.ts
  • frontend/packages/integration-tests/tests/crud/image-pull-secret.cy.ts
  • frontend/packages/integration-tests/tests/crud/other-routes.cy.ts
  • frontend/packages/integration-tests/tests/crud/quotas.cy.ts
  • frontend/packages/integration-tests/tests/crud/roles-rolebindings.cy.ts
  • frontend/packages/integration-tests/views/rolebindings.ts
  • frontend/public/components/api-explorer.tsx
  • frontend/public/components/cluster-settings/cluster-settings.tsx
  • frontend/public/components/error.tsx
  • frontend/public/components/factory/table.tsx
  • frontend/public/components/utils/actions-menu.tsx
  • frontend/public/components/utils/kebab.tsx
💤 Files with no reviewable changes (6)
  • frontend/packages/integration-tests/tests/crud/quotas.cy.ts
  • frontend/packages/integration-tests/views/rolebindings.ts
  • frontend/packages/integration-tests/tests/crud/image-pull-secret.cy.ts
  • frontend/packages/integration-tests/tests/crud/other-routes.cy.ts
  • frontend/packages/integration-tests/tests/crud/add-storage-crud.cy.ts
  • frontend/packages/integration-tests/tests/crud/roles-rolebindings.cy.ts

Comment thread frontend/e2e/pages/role-binding-page.ts
Comment thread frontend/e2e/tests/console/crud/add-storage-crud.spec.ts Outdated
Comment thread frontend/e2e/tests/console/crud/image-pull-secret.spec.ts Outdated
@rhamilto

rhamilto commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

/label tide/merge-method-squash

@openshift-ci openshift-ci Bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 4, 2026
@rhamilto rhamilto removed the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/pages/details-page.ts`:
- Around line 31-34: clickPageAction currently only clicks action nodes found
via this.page.getByTestId(actionName), but tests still need to support legacy
selectors; update clickPageAction to try the test-id selector first and if no
element is found fall back to a selector that targets the legacy attribute
(data-test-action) before calling this.robustClick. Use the existing method
names to locate the change: modify async clickPageAction(actionName: string) to
call this.page.getByTestId(actionName) and, if that returns no match, use
this.page.locator(`[data-test-action="${actionName}"]`) (or equivalent) and then
invoke this.robustClick on whichever locator exists; keep the initial click on
the actions-menu-button using
this.robustClick(this.page.getByTestId('actions-menu-button')).

In `@frontend/public/components/utils/kebab.tsx`:
- Line 73: The data-test attribute currently uses option.label which may be a
ReactNode and thus unstable; change the assignment in the component that renders
data-test (in kebab.tsx where you build the option element) to prefer a
deterministic string: use option.labelKey (translated via t(option.labelKey,
option.labelKind) if available) and only fall back to option.label when typeof
option.label === 'string'; ensure the final data-test value is always a string
(e.g., t(option.labelKey, option.labelKind) || (typeof option.label === 'string'
? option.label : undefined)) so selectors remain stable.
🪄 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: 31e8ea55-7764-4bad-a1fd-35f7ec354645

📥 Commits

Reviewing files that changed from the base of the PR and between 9ed5194 and ee71bda.

⛔ Files ignored due to path filters (1)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (34)
  • .claude/migration-context.md
  • .claude/skills/migrate-cypress/SKILL.md
  • frontend/e2e/clients/kubernetes-client.ts
  • frontend/e2e/fixtures/cleanup-fixture.ts
  • frontend/e2e/pages/alertmanager-page.ts
  • frontend/e2e/pages/base-page.ts
  • frontend/e2e/pages/details-page.ts
  • frontend/e2e/pages/list-page.ts
  • frontend/e2e/pages/role-binding-page.ts
  • frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts
  • frontend/e2e/tests/console/crud/add-storage-crud.spec.ts
  • frontend/e2e/tests/console/crud/image-pull-secret.spec.ts
  • frontend/e2e/tests/console/crud/other-routes.spec.ts
  • frontend/e2e/tests/console/crud/quotas.spec.ts
  • frontend/e2e/tests/console/crud/roles-rolebindings.spec.ts
  • frontend/e2e/utils/a11y.ts
  • frontend/package.json
  • frontend/packages/console-app/src/components/nav/NavHeader.tsx
  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuContent.tsx
  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuItem.tsx
  • frontend/packages/console-shared/src/components/breadcrumbs/Breadcrumbs.tsx
  • frontend/packages/integration-tests/tests/crud/add-storage-crud.cy.ts
  • frontend/packages/integration-tests/tests/crud/image-pull-secret.cy.ts
  • frontend/packages/integration-tests/tests/crud/other-routes.cy.ts
  • frontend/packages/integration-tests/tests/crud/quotas.cy.ts
  • frontend/packages/integration-tests/tests/crud/roles-rolebindings.cy.ts
  • frontend/packages/integration-tests/views/rolebindings.ts
  • frontend/public/components/api-explorer.tsx
  • frontend/public/components/cluster-settings/cluster-settings.tsx
  • frontend/public/components/error.tsx
  • frontend/public/components/factory/table.tsx
  • frontend/public/components/storage/attach-pvc-storage.tsx
  • frontend/public/components/utils/actions-menu.tsx
  • frontend/public/components/utils/kebab.tsx
💤 Files with no reviewable changes (6)
  • frontend/packages/integration-tests/views/rolebindings.ts
  • frontend/packages/integration-tests/tests/crud/quotas.cy.ts
  • frontend/packages/integration-tests/tests/crud/add-storage-crud.cy.ts
  • frontend/packages/integration-tests/tests/crud/roles-rolebindings.cy.ts
  • frontend/packages/integration-tests/tests/crud/other-routes.cy.ts
  • frontend/packages/integration-tests/tests/crud/image-pull-secret.cy.ts
✅ Files skipped from review due to trivial changes (8)
  • frontend/public/components/storage/attach-pvc-storage.tsx
  • frontend/public/components/cluster-settings/cluster-settings.tsx
  • frontend/packages/console-shared/src/components/breadcrumbs/Breadcrumbs.tsx
  • frontend/public/components/factory/table.tsx
  • .claude/skills/migrate-cypress/SKILL.md
  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuItem.tsx
  • frontend/public/components/api-explorer.tsx
  • frontend/packages/console-app/src/components/nav/NavHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (16)
  • frontend/public/components/error.tsx
  • frontend/package.json
  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuContent.tsx
  • frontend/e2e/clients/kubernetes-client.ts
  • frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts
  • frontend/e2e/pages/role-binding-page.ts
  • frontend/e2e/utils/a11y.ts
  • frontend/e2e/fixtures/cleanup-fixture.ts
  • frontend/e2e/pages/alertmanager-page.ts
  • .claude/migration-context.md
  • frontend/e2e/pages/base-page.ts
  • frontend/e2e/tests/console/crud/quotas.spec.ts
  • frontend/e2e/tests/console/crud/image-pull-secret.spec.ts
  • frontend/e2e/pages/list-page.ts
  • frontend/e2e/tests/console/crud/roles-rolebindings.spec.ts
  • frontend/e2e/tests/console/crud/other-routes.spec.ts

Comment thread frontend/e2e/pages/details-page.ts
Comment thread frontend/public/components/utils/kebab.tsx Outdated
@rhamilto

rhamilto commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

/retest

@rhamilto rhamilto force-pushed the CONSOLE-5278 branch 3 times, most recently from 0a9a79b to 8784104 Compare June 8, 2026 13:38
@rhamilto

rhamilto commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

tech debt
/label px-approved
/label docs-approved

@openshift-ci openshift-ci Bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Jun 8, 2026
@rhamilto

rhamilto commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

/label tide/merge-method-squash

@openshift-ci openshift-ci Bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 8, 2026
@rhamilto rhamilto changed the title [WIP] CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright Jun 8, 2026
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2026
@rhamilto

rhamilto commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

/test e2e-playwright

Comment thread frontend/e2e/pages/list-page.ts Outdated
'[data-ouia-component-id="DataViewFilters"]',
);
private readonly namespaceDropdown = this.page.getByTestId('namespace-bar-dropdown');
private readonly legacyResourceRows = this.page.locator('[data-test-rows="resource-row"]');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can add data-test to frontend/public/components/factory/table.tsx and use getByTestId

Comment thread frontend/e2e/pages/list-page.ts Outdated
);
private readonly namespaceDropdown = this.page.getByTestId('namespace-bar-dropdown');
private readonly legacyResourceRows = this.page.locator('[data-test-rows="resource-row"]');
private readonly legacyNameFilter = this.page.getByTestId('name-filter-input');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why legacy?

Comment thread frontend/e2e/pages/list-page.ts Outdated
const filterToggle = this.dataViewFilters.locator('.pf-v6-c-menu-toggle').first();
await this.robustClick(filterToggle);
await this.page.locator('.pf-v6-c-menu__list-item', { hasText: 'Name' }).click();
await this.nameFilterInput.waitFor({ state: 'visible' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed, fill will auto-await

Comment thread frontend/e2e/pages/list-page.ts Outdated
}

async legacyClickRowByName(resourceName: string): Promise<void> {
const link = this.page.locator(`a[data-test-id="${resourceName}"]`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add data-test?

Comment thread frontend/e2e/pages/list-page.ts Outdated

async clickCreateYAMLDropdownButton(): Promise<void> {
await this.robustClick(this.createButton);
const yamlMenuItem = this.page.locator('[data-test-dropdown-menu="yaml"]');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have access to the related element and add data-test

Comment on lines +44 to +45
await page.getByTestId('resource-sidebar').waitFor({ state: 'visible' });
await page.getByTestId('code-editor').waitFor({ state: 'visible' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not needed

await page.getByTestId('pvc-size').fill(pvcSize);
await page.getByTestId('mount-path').fill(mountPath);
await page.getByTestId('save-changes').click();
await expect(page.getByTestId('yaml-error')).not.toBeAttached();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's hard to say what we're doing here. Either we wrap it in a function or we create named variables in the DetailsPage

Comment on lines +30 to +32
await listPage.waitForListLoad();
await listPage.selectProject(namespace);
await listPage.waitForListLoad();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this should be a single method that leverages Playwirght's auto-awaiting

await route.waitFor(page);
}

await testA11y(page, `route ${route.path.replace(/\//g, ' ')}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about why we test a11y only here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure. Is legacy.

Comment on lines +23 to +37
try {
await k8sClient.deleteClusterCustomResource(
'quota.openshift.io',
'v1',
'clusterresourcequotas',
clusterQuotaName,
);
} catch {
// may already be deleted
}
try {
await k8sClient.deleteNamespace(namespace);
} catch {
// may already be deleted
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the k8sClient already swallow 404s

@openshift-ci

openshift-ci Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Comment thread frontend/e2e/pages/list-page.ts Outdated
Comment on lines +16 to +29
async waitForListLoad(): Promise<void> {
await this.waitForLoadingComplete();
await this.page
.getByTestId('page-heading')
.waitFor({ state: 'visible', timeout: 60_000 });
}

async waitForTableLoad(): Promise<void> {
await this.waitForLoadingComplete();
await this.dataViewTable
.or(this.resourceRows.first())
.first()
.waitFor({ state: 'visible', timeout: 60_000 });
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these functions are not necessary

Comment thread frontend/e2e/pages/list-page.ts Outdated
}

async filterByCheckbox(filterName: string, checkboxLabel: string): Promise<void> {
await this.dataViewFilters.waitFor({ state: 'visible', timeout: 60_000 });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

await yamlViewInput.click();
}

await page.getByTestId('code-editor').waitFor({ state: 'visible' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

Comment thread frontend/e2e/pages/list-page.ts Outdated
}

async filterByCheckbox(filterName: string, checkboxLabel: string): Promise<void> {
await this.dataViewFilters.waitFor({ state: 'visible', timeout: 60_000 });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

await yamlViewInput.click();
}

await page.getByTestId('code-editor').waitFor({ state: 'visible' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double check that this is actually needed


await test.step('Add storage via Actions dropdown', async () => {
const details = new DetailsPage(page);
await details.waitForPageLoad();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed (same for all other occurrences)


await page.getByTestId('item-create').click();

await page.getByTestId('save-changes').waitFor({ state: 'visible' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double check whether this is needed

Comment thread frontend/e2e/pages/list-page.ts Outdated
Comment on lines +16 to +29
async waitForListLoad(): Promise<void> {
await this.waitForLoadingComplete();
await this.page
.getByTestId('page-heading')
.waitFor({ state: 'visible', timeout: 60_000 });
}

async waitForTableLoad(): Promise<void> {
await this.waitForLoadingComplete();
await this.dataViewTable
.or(this.resourceRows.first())
.first()
.waitFor({ state: 'visible', timeout: 60_000 });
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be removed, they are usually followed by actions

@rhamilto

rhamilto commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

Comments addressed by Claude. Latest feedback from Claude re: remaining waitFors:

All three are legitimate:

role-binding-page.ts:48 — loading-indicator.waitFor({ state: 'detached' }) after robustClick(saveButton). Waiting for loading to finish after save. No action follows on the same element.

other-routes.spec.ts:149 — route.waitFor(page) is calling a route config callback function, not the Playwright waitFor method. It's a plain function call.

other-routes.spec.ts:166 — toggle.waitFor({ state: 'visible' }) followed by toggle.getAttribute('id'). getAttribute is a read, not an action — it does NOT auto-wait for visibility. This waitFor is necessary.

All three are compliant. No violations in branch-added files.

rhamilto and others added 12 commits June 9, 2026 13:34
Playwright action methods (fill, click, check, clear) and robustClick
auto-wait for element actionability. Explicit waitFor({ state: 'visible' })
immediately before an action on the same element is redundant.

- Remove DetailsPage.waitForPageLoad() method along with the unused
  skeletonLoader and resourceTitle fields it depended on
- Remove waitForLoad parameter from clickResourceRow (never called
  with false, and robustClick already auto-waits)
- Remove redundant waitFor before robustClick in clickKebabAction
  and clickResourceRow
- Remove redundant waitFor calls in cluster-settings-page, navigation,
  oauth-page, web-terminal-page, and favorites.spec.ts
- Fix migration docs: deleteClusterCustomResource does not exist in
  KubernetesClient; only deleteNamespace and deleteCustomResource
  swallow 404 errors
- Update migration-context.md and migrate-cypress skill with
  auto-awaiting rules to prevent redundant waitFor in future migrations

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrate all 5 CRUD Cypress test files to Playwright:

- other-routes.cy.ts → other-routes.spec.ts (error page, API explorer,
  cluster settings routes, perspective query parameters)
- quotas.cy.ts → quotas.spec.ts (ResourceQuota and ClusterResourceQuota
  creation via YAML editor, list filtering across projects)
- roles-rolebindings.cy.ts → roles-rolebindings.spec.ts (Role and
  ClusterRole creation via YAML editor, RoleBinding and
  ClusterRoleBinding creation via form, rules table columns, breadcrumb
  project dropdown restoration)
- image-pull-secret.cy.ts → image-pull-secret.spec.ts (image pull
  secret creation via form, whitespace trimming validation)
- add-storage-crud.cy.ts → add-storage-crud.spec.ts (workload creation
  via YAML editor, add storage via Actions dropdown, volume table
  verification)

Infrastructure:
- Extract shared Monaco editor helpers to base-page.ts
- Add cluster-scoped create/delete to KubernetesClient
- Add cluster resource tracking to cleanup fixture
- Add data-test alongside data-test-id on Breadcrumbs, actions menu
  button, kebab items, ActionMenuItem, and ActionMenuContent
- Add data-test to attach-pvc-storage submit button
- New page objects: RoleBindingPage, ListPage
- Extend DetailsPage with clickPageAction and getBreadcrumb

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CI fixes:
- quotas.spec.ts: use waitForTableLoad instead of waitForListLoad
  before filterByName to ensure DataView is loaded
- quotas.spec.ts: use selectAllProjects() instead of
  selectProject('All Projects') which fails in search filter
- ListPage: add waitForTableLoad, selectAllProjects methods
- ListPage.filterByName: check if filter input is already visible
  before clicking the DataViewFilters dropdown toggle

CodeRabbit feedback:
- DetailsPage.clickPageAction: fall back to data-test-action when
  data-test is absent for legacy action menu items
- kebab.tsx: only set data-test when option.label is a string to
  avoid [object Object] values from ReactNode labels

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add serial mode to ensure all tests run in the same worker sharing
the beforeAll namespace. Use unique resource names with Date.now()
suffix to prevent strict mode violations from leftover resources
of prior test runs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use kindForReference as fallback text when the model hasn't loaded
yet, ensuring the accordion toggle button always has discernible text.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Import expect from e2e/fixtures instead of @playwright/test in a11y.ts
- Use DetailsPage.getBreadcrumb() instead of raw locators in quotas.spec.ts
- Use listPage.selectAllProjects() consistently in roles-rolebindings.spec.ts
- Add serial mode to roles-rolebindings.spec.ts for CI stability
- Add data-test attributes to volumes-table.tsx and use getByTestId in spec

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pull in improvements from PR 16556 so this PR can merge first and
16556 can rebase cleanly without conflicts.

- base-page: add waitForFunction guard to Monaco editor helpers
- details-page: simplify clickPageAction, use getByTestId in clickKebabAction
- list-page: add legacy locators/methods, getters, kebab actions,
  showSystemSwitch toggle, and waitForTableLoad legacy fallback
- kubernetes-client: add createConfigMap, createSecret, mergePatchResource,
  annotateConfigMap, labelConfigMap
- cleanup-fixture: align trackClusterCustomResource type default
- crd-test-utils: fix expect import to use fixtures

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove redundant waitFor() calls before Playwright actions that auto-wait,
remove unnecessary try/catch and .catch(() => {}) around k8sClient cleanup
calls that already swallow 404s, replace legacy test attribute selectors
with getByTestId(), rename page object methods to remove "legacy" prefix,
add data-test attribute to SimpleDropdown items in list-page.tsx, and
consolidate inline navigation sequences into page object methods. Update
migration context and skill docs to prevent these patterns in future
migrations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Playwright actions (fill, click, check, etc.) and robustClick auto-wait
for element actionability. Remove waitFor calls that immediately precede
these actions in crd-test-utils.ts and other-routes.spec.ts. Fix
alertmanager-page.ts to import expect from fixtures instead of
@playwright/test. Update migration-context.md Waits and Retries table
to guide toward passing timeout to actions/assertions rather than using
standalone waitFor.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove waitForListLoad(), waitForTableLoad(), and dataViewFilters.waitFor()
from list-page.ts. Remove code-editor and save-changes waitFor calls from
spec files. Playwright actions auto-wait for actionability, and
getEditorContent() has its own internal waitForFunction.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Playwright auto-waits for actionability on subsequent actions and
assertions auto-retry, making explicit page-load guards redundant.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This branch introduces KubernetesClient.deleteClusterCustomResource()
which swallows 404s like deleteNamespace and deleteCustomResource.
Update migration-context.md and SKILL.md to include it in the
no-try/catch cleanup guidance.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rhamilto

rhamilto commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

/retest

@openshift-ci

openshift-ci Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@rhamilto: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-images 231fadc link true /test okd-scos-images
ci/prow/analyze 231fadc link true /test analyze
ci/prow/frontend 231fadc link true /test frontend
ci/prow/images 231fadc link true /test images

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/shared Related to console-shared docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. kind/cypress Related to Cypress e2e integration testing px-approved Signifies that Product Support has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants