[WIP] CONSOLE-5277: Migrate console e2e CRUD tests to Playwright#16556
Open
rhamilto wants to merge 20 commits into
Open
[WIP] CONSOLE-5277: Migrate console e2e CRUD tests to Playwright#16556rhamilto wants to merge 20 commits into
rhamilto wants to merge 20 commits into
Conversation
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>
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@rhamilto: This pull request references CONSOLE-5277 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. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
rhamilto
changed the title
Contributor
|
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:
WalkthroughMigrates E2E from Cypress to Playwright: adds Kubernetes client merge-patch and cluster CR helpers, Playwright page objects and utilities, accessibility/i18n helpers, many Playwright specs, UI test-selector tweaks, and removes legacy Cypress specs. ChangesE2E Test Infrastructure Modernization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
component/shared
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/clients/kubernetes-client.ts`:
- Around line 403-441: The PATCH request in mergePatchResource currently can
hang because the https.request has no timeout; update the request created in
mergePatchResource to fail fast by calling req.setTimeout(...) (choose a
sensible timeout, e.g. 30_000 ms or a configurable constant) and in the timeout
handler call req.destroy(new Error('Request timed out')) (or similar) so the
Promise rejects via the existing req.on('error') handler; ensure you add the
timeout before writing the body and clean up the timeout on success/failure.
In `@frontend/e2e/pages/navigation.ts`:
- Around line 29-31: The locator read of aria-expanded on sectionButton can race
with rendering; before calling sectionButton.getAttribute('aria-expanded') wait
for the button to be visible (e.g. await sectionButton.waitFor({ state:
'visible' }) or an equivalent visibility wait) so the attribute read is stable;
update the code that references sectionButton and getAttribute('aria-expanded')
to perform this visibility wait first.
In `@frontend/e2e/tests/console/crud/annotations.spec.ts`:
- Line 12: The tests use a shared constant configmapName ('example') which can
cause cross-test races; update the tests in annotations.spec.ts to generate a
unique per-test ConfigMap name inside each test (e.g., build from test title or
append a UUID/timestamp) and replace the module-level configmapName usage with
that per-test variable throughout each test flow (create/verify/delete). Also
register the created ConfigMap with the Playwright per-test cleanup fixture so
deletion is scoped to the test; update references to functions/assertions that
currently read the module-level configmapName to use the new per-test variable
instead.
🪄 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: a639b307-4f1a-4d77-af7a-ce240c888833
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
frontend/e2e/clients/kubernetes-client.tsfrontend/e2e/fixtures/cleanup-fixture.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/pages/modal-page.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/pages/yaml-editor-page.tsfrontend/e2e/tests/console/crud/annotations.spec.tsfrontend/e2e/tests/console/crud/bulk-create-resources.spec.tsfrontend/e2e/tests/console/crud/customresourcedefinition.spec.tsfrontend/e2e/tests/console/crud/environment.spec.tsfrontend/e2e/tests/console/crud/labels.spec.tsfrontend/e2e/tests/console/crud/namespace-crud.spec.tsfrontend/e2e/tests/console/crud/resource-crud.spec.tsfrontend/e2e/utils/a11y.tsfrontend/e2e/utils/i18n.tsfrontend/e2e/utils/test-name.tsfrontend/package.jsonfrontend/packages/console-shared/src/components/actions/menu/ActionMenuItem.tsxfrontend/packages/console-shared/src/components/breadcrumbs/Breadcrumbs.tsxfrontend/packages/console-shared/src/components/modals/CreateNamespaceModal.tsxfrontend/packages/integration-tests/tests/crud/annotations.cy.tsfrontend/packages/integration-tests/tests/crud/bulk-create-resources.cy.tsfrontend/packages/integration-tests/tests/crud/customresourcedefinition.cy.tsfrontend/packages/integration-tests/tests/crud/environment.cy.tsfrontend/packages/integration-tests/tests/crud/labels.cy.tsfrontend/packages/integration-tests/tests/crud/namespace-crud.cy.tsfrontend/packages/integration-tests/tests/crud/resource-crud.cy.tsfrontend/public/components/modals/delete-modal.tsxfrontend/public/components/modals/delete-namespace-modal.tsxfrontend/public/components/modals/labels-modal.tsxfrontend/public/components/modals/tags.tsxfrontend/public/components/utils/name-value-editor.tsx
💤 Files with no reviewable changes (7)
- frontend/packages/integration-tests/tests/crud/annotations.cy.ts
- frontend/packages/integration-tests/tests/crud/customresourcedefinition.cy.ts
- frontend/packages/integration-tests/tests/crud/environment.cy.ts
- frontend/packages/integration-tests/tests/crud/namespace-crud.cy.ts
- frontend/packages/integration-tests/tests/crud/labels.cy.ts
- frontend/packages/integration-tests/tests/crud/resource-crud.cy.ts
- frontend/packages/integration-tests/tests/crud/bulk-create-resources.cy.ts
| import { YamlEditorPage } from '../../../pages/yaml-editor-page'; | ||
| import { generateTestName } from '../../../utils/test-name'; | ||
|
|
||
| const configmapName = 'example'; |
Contributor
There was a problem hiding this comment.
Use per-test ConfigMap names to avoid cross-test collisions.
Both tests operate on configmapName = 'example' in the same shared namespace. In parallel/failed-run scenarios this can race (create/delete conflicts) and make the suite flaky. Generate a unique ConfigMap name inside each test (or include test-specific suffix) and use that value throughout the test steps.
Suggested patch pattern
-const configmapName = 'example';
@@
test('creates, edits, updates, and deletes annotations', async ({ page }) => {
+ const configmapName = `${generateTestName()}-cm`;
@@
test('disables Save when annotations change externally', async ({ page, k8sClient }) => {
+ const configmapName = `${generateTestName()}-cm`;Based on learnings: “Use the per-test Playwright cleanup fixture when each test creates its own resources and needs isolated create/cleanup.”
Also applies to: 39-234
🤖 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/annotations.spec.ts` at line 12, The tests
use a shared constant configmapName ('example') which can cause cross-test
races; update the tests in annotations.spec.ts to generate a unique per-test
ConfigMap name inside each test (e.g., build from test title or append a
UUID/timestamp) and replace the module-level configmapName usage with that
per-test variable throughout each test flow (create/verify/delete). Also
register the created ConfigMap with the Playwright per-test cleanup fixture so
deletion is scoped to the test; update references to functions/assertions that
currently read the module-level configmapName to use the new per-test variable
instead.
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>
Member
Author
|
/retest |
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>
Member
Author
|
/test e2e-playwright |
Member
Author
|
/test e2e-gcp-console |
- 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>
1 task
Migrates the parameterized resource CRUD lifecycle test (21 resource types × 6 steps each) from Cypress to Playwright with full feature parity including a11y checks, i18n pseudolocalization verification, and parallel execution support. New page objects: ListPage, ModalPage, YamlEditorPage New utilities: a11y (axe-core), i18n (pseudolocalization) Extended: BasePage (YAML editor helpers), DetailsPage (breadcrumbs, page actions), KubernetesClient (cluster-scoped CRUD), CleanupFixture (cluster-scoped resource tracking) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrates the namespace CRUD lifecycle test (3 tests) covering namespace list/create/delete, nav link namespace preservation, and breadcrumb namespace preservation. Corrects the "All Projects" restoration assertion — the Console preserves the last-used namespace, not "All Projects", when navigating back via nav link or breadcrumb. Extended: ListPage (clickFirstLinkInFirstRow, getFirstCellText, selectAllProjects, selectProject with system namespace toggle), Navigation (clickNavLink for mid-test nav), ModalPage (wait for enabled before submit), DetailsPage (breadcrumb legacy selector fix) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ight Migrates the CRD CRUD lifecycle test covering CRD creation via YAML editor, viewing instances, creating a custom resource, and deleting the CRD via kebab action. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mirrors the Cypress testName helper — generates a random lowercase alphabetic string prefixed with "test" that is safe for use in Kubernetes resource names, CRD shortNames, and namespace names without any post-processing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrates the annotations CRUD test (2 tests) covering annotation create/edit/update/delete via the modal, and external annotation change detection (save button disabled + info alert). Adds data-test attributes alongside legacy data-test-action, data-test-id attributes in ActionMenuItem, Breadcrumbs, delete-modal, delete-namespace-modal, CreateNamespaceModal, and tags modal so all page objects use getByTestId() exclusively. Extended: KubernetesClient (createConfigMap, annotateConfigMap via raw merge-patch HTTP request) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrates the label editing test (2 tests) covering label add/verify/search-navigation via the modal, and external label change detection (save button disabled + info alert). Adds data-test attribute to labels modal cancel button. Refactors KubernetesClient merge-patch into a shared mergePatchResource helper used by both annotateConfigMap and new labelConfigMap. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrates the environment variable editor test (3 tests) covering add/delete plain env vars, env vars from ConfigMap, and env vars from Secret. Uses click-based navigation (sidebar → list → row → tab) instead of direct URL navigation to avoid Console "Model does not exist" errors when the SPA model cache is cold. Adds data-test attributes to env-prefix and pairs-list__delete-from-btn in name-value-editor. Extended: KubernetesClient (createSecret) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrates the bulk import test (3 tests) covering duplicate YAML validation, missing namespace server validation, and successful multi-resource import with result verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- mergePatchResource: add proxy agent support for CI environments - setEditorContent/getEditorContent: wait for Monaco model readiness before reading/writing to avoid silent no-ops - testI18n: add null check for browser reference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- mergePatchResource: add 30s request timeout to fail fast on stalled connections instead of hanging indefinitely - clickNavLink: wait for nav section button visibility before reading aria-expanded to avoid race with sidebar rendering Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds data-test="modal-cancel-action" alongside the existing data-test-id on the PVC delete modal cancel button so getByTestId() can find it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Kebab and Actions menu items use data-test-action, not data-test. Using getByTestId matched the wrong attribute, causing timeouts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add data-test alongside existing data-test-action attributes so Playwright tests can use page.getByTestId() per migration guidelines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add data-test="modal-cancel-action" to 13 modal cancel buttons so Playwright's getByTestId() (configured for data-test) can locate them. Fix CRD test: remove conflicting listKind and use retry-based navigation to the instances page to handle API discovery timing. Fix labels test: use regex alternation (~ |%7E) instead of a character class for the tilde-encoded URL segment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
/retest |
1 similar comment
Member
Author
|
/retest |
Contributor
|
@rhamilto: The following tests 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR includes changes from #16544, which should merge first.
Analysis / Root cause:
CONSOLE-5277 — Migrate 7 Cypress test files (20 tests) from
packages/integration-tests/tests/crud/to Playwright undere2e/tests/console/crud/.Solution description:
Migrates all CRUD Cypress tests to Playwright with full feature parity:
resource-crud.cy.tsresource-crud.spec.tsnamespace-crud.cy.tsnamespace-crud.spec.tscustomresourcedefinition.cy.tscustomresourcedefinition.spec.tsannotations.cy.tsannotations.spec.tslabels.cy.tslabels.spec.tsenvironment.cy.tsenvironment.spec.tsbulk-create-resources.cy.tsbulk-create-resources.spec.tsKey infrastructure added:
ListPage,ModalPage,YamlEditorPage(new); extendedBasePage,DetailsPage,NavigationtestA11y(axe-core a11y),testI18n(pseudolocalization),generateTestName(DNS-safe names)createConfigMap,createSecret,createClusterCustomResource,deleteClusterCustomResource,annotateConfigMap,labelConfigMap,mergePatchResourcetrackClusterCustomResourcefor non-namespaced resource cleanupdata-testattributes alongside legacydata-test-id/data-test-actioninActionMenuItem,Breadcrumbs, modal cancel buttons,name-value-editor@axe-core/playwrightfor accessibility testingAll tests run in parallel with
test.describe.configure({ mode: 'parallel' })where applicable (resource-crud runs 21 tests across 4 workers in ~3min vs ~6min serial).Screenshots / screen recording:
N/A — test migration, no UI changes
Test setup:
Test cases:
npx tsc --noEmit -p e2e/tsconfig.json— zero errorsyarn eslinton all new/modified files — zero warningsBrowser conformance:
Summary by CodeRabbit
Tests
Chores
New Test Helpers
Style