CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright#16544
CONSOLE-5278: Migrate console CRUD Cypress tests to Playwright#16544rhamilto wants to merge 12 commits into
Conversation
|
@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. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR updates Cypress→Playwright migration docs/selectors, adds Monaco editor helpers, cluster-CR support, page objects, accessibility tooling, many Playwright test suites, ChangesCypress→Playwright E2E migration with infrastructure and test coverage expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/e2e/tests/console/crud/quotas.spec.ts (2)
22-38: 💤 Low valueConsider the
deleteClusterCustomResourcewrapper for consistency.The roles spec deletes cluster-scoped resources via
k8sClient.deleteClusterCustomResource(...), while here you callk8sClient.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 winRemove dead
browser.newPage()/close()code.These two lines open and immediately close a blank page, accomplishing nothing, and pull an unused
browserfixture intobeforeAll. 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
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
.claude/migration-context.md.claude/skills/migrate-cypress/SKILL.mdfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/fixtures/cleanup-fixture.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/pages/role-binding-page.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crud/add-storage-crud.spec.tsfrontend/e2e/tests/console/crud/image-pull-secret.spec.tsfrontend/e2e/tests/console/crud/other-routes.spec.tsfrontend/e2e/tests/console/crud/quotas.spec.tsfrontend/e2e/tests/console/crud/roles-rolebindings.spec.tsfrontend/e2e/utils/a11y.tsfrontend/package.jsonfrontend/packages/console-app/src/components/nav/NavHeader.tsxfrontend/packages/console-shared/src/components/breadcrumbs/Breadcrumbs.tsxfrontend/packages/integration-tests/tests/crud/add-storage-crud.cy.tsfrontend/packages/integration-tests/tests/crud/image-pull-secret.cy.tsfrontend/packages/integration-tests/tests/crud/other-routes.cy.tsfrontend/packages/integration-tests/tests/crud/quotas.cy.tsfrontend/packages/integration-tests/tests/crud/roles-rolebindings.cy.tsfrontend/packages/integration-tests/views/rolebindings.tsfrontend/public/components/api-explorer.tsxfrontend/public/components/cluster-settings/cluster-settings.tsxfrontend/public/components/error.tsxfrontend/public/components/factory/table.tsxfrontend/public/components/utils/actions-menu.tsxfrontend/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
|
/label tide/merge-method-squash |
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
.claude/migration-context.md.claude/skills/migrate-cypress/SKILL.mdfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/fixtures/cleanup-fixture.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/pages/role-binding-page.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crud/add-storage-crud.spec.tsfrontend/e2e/tests/console/crud/image-pull-secret.spec.tsfrontend/e2e/tests/console/crud/other-routes.spec.tsfrontend/e2e/tests/console/crud/quotas.spec.tsfrontend/e2e/tests/console/crud/roles-rolebindings.spec.tsfrontend/e2e/utils/a11y.tsfrontend/package.jsonfrontend/packages/console-app/src/components/nav/NavHeader.tsxfrontend/packages/console-shared/src/components/actions/menu/ActionMenuContent.tsxfrontend/packages/console-shared/src/components/actions/menu/ActionMenuItem.tsxfrontend/packages/console-shared/src/components/breadcrumbs/Breadcrumbs.tsxfrontend/packages/integration-tests/tests/crud/add-storage-crud.cy.tsfrontend/packages/integration-tests/tests/crud/image-pull-secret.cy.tsfrontend/packages/integration-tests/tests/crud/other-routes.cy.tsfrontend/packages/integration-tests/tests/crud/quotas.cy.tsfrontend/packages/integration-tests/tests/crud/roles-rolebindings.cy.tsfrontend/packages/integration-tests/views/rolebindings.tsfrontend/public/components/api-explorer.tsxfrontend/public/components/cluster-settings/cluster-settings.tsxfrontend/public/components/error.tsxfrontend/public/components/factory/table.tsxfrontend/public/components/storage/attach-pvc-storage.tsxfrontend/public/components/utils/actions-menu.tsxfrontend/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
|
/retest |
0a9a79b to
8784104
Compare
|
tech debt |
|
/label tide/merge-method-squash |
|
/test e2e-playwright |
| '[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"]'); |
There was a problem hiding this comment.
we can add data-test to frontend/public/components/factory/table.tsx and use getByTestId
| ); | ||
| 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'); |
| 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' }); |
There was a problem hiding this comment.
not needed, fill will auto-await
| } | ||
|
|
||
| async legacyClickRowByName(resourceName: string): Promise<void> { | ||
| const link = this.page.locator(`a[data-test-id="${resourceName}"]`); |
|
|
||
| async clickCreateYAMLDropdownButton(): Promise<void> { | ||
| await this.robustClick(this.createButton); | ||
| const yamlMenuItem = this.page.locator('[data-test-dropdown-menu="yaml"]'); |
There was a problem hiding this comment.
I think we have access to the related element and add data-test
| await page.getByTestId('resource-sidebar').waitFor({ state: 'visible' }); | ||
| await page.getByTestId('code-editor').waitFor({ state: 'visible' }); |
| 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(); |
There was a problem hiding this comment.
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
| await listPage.waitForListLoad(); | ||
| await listPage.selectProject(namespace); | ||
| await listPage.waitForListLoad(); |
There was a problem hiding this comment.
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, ' ')}`); |
There was a problem hiding this comment.
I'm curious about why we test a11y only here
There was a problem hiding this comment.
Not entirely sure. Is legacy.
| 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 | ||
| } |
There was a problem hiding this comment.
I think that the k8sClient already swallow 404s
|
[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 |
| 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 }); | ||
| } |
There was a problem hiding this comment.
these functions are not necessary
| } | ||
|
|
||
| async filterByCheckbox(filterName: string, checkboxLabel: string): Promise<void> { | ||
| await this.dataViewFilters.waitFor({ state: 'visible', timeout: 60_000 }); |
| await yamlViewInput.click(); | ||
| } | ||
|
|
||
| await page.getByTestId('code-editor').waitFor({ state: 'visible' }); |
| } | ||
|
|
||
| async filterByCheckbox(filterName: string, checkboxLabel: string): Promise<void> { | ||
| await this.dataViewFilters.waitFor({ state: 'visible', timeout: 60_000 }); |
| await yamlViewInput.click(); | ||
| } | ||
|
|
||
| await page.getByTestId('code-editor').waitFor({ state: 'visible' }); |
There was a problem hiding this comment.
double check that this is actually needed
|
|
||
| await test.step('Add storage via Actions dropdown', async () => { | ||
| const details = new DetailsPage(page); | ||
| await details.waitForPageLoad(); |
There was a problem hiding this comment.
not needed (same for all other occurrences)
|
|
||
| await page.getByTestId('item-create').click(); | ||
|
|
||
| await page.getByTestId('save-changes').waitFor({ state: 'visible' }); |
There was a problem hiding this comment.
double check whether this is needed
| 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 }); | ||
| } |
There was a problem hiding this comment.
these should be removed, they are usually followed by actions
|
Comments addressed by Claude. Latest feedback from Claude re: remaining |
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>
|
/retest |
|
@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. |
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:
e2e/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.e2e/tests/console/crud/quotas.spec.ts— creates ResourceQuota and ClusterResourceQuota via YAML editor, verifies list filtering across All Projects and project-scoped viewse2e/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 restoratione2e/tests/console/crud/image-pull-secret.spec.ts— creates image pull secret via form, verifies whitespace trimminge2e/tests/console/crud/add-storage-crud.spec.ts— creates each workload type via YAML editor, adds storage, verifies volume tableInfrastructure additions:
getEditorContent/setEditorContent) extracted tobase-page.tswithwaitForFunctionguards for reliable editor readinessKubernetesClient(includingdeleteClusterCustomResourcewhich swallows 404s)createConfigMap,createSecret,mergePatchResource,annotateConfigMap,labelConfigMapmethods added toKubernetesClient(shared with PR [WIP] CONSOLE-5277: Migrate console e2e CRUD tests to Playwright #16556)data-testattributes added alongsidedata-test-idon Breadcrumbs, actions menu button, kebab action items, action menu items, perspective switcher, volumes table, and other UI elementsRoleBindingPage, extendedListPage(filterByCheckbox, legacy locators/methods, kebab actions,showSystemSwitchtoggle), extendedDetailsPage(getBreadcrumb, clickPageAction, clickKebabAction)a11y.ts) using@axe-core/playwrightDetailsPage.waitForPageLoad()callers — Playwright actions and assertions auto-wait, making the explicit page-load wait redundantdeleteClusterCustomResourcein the no-try/catch cleanup guidanceStacked on PR #16583 (
playwright-waitfor-cleanup) which removes redundantwaitForcalls andDetailsPage.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 totestIsolation:falsecarrying state from prior tests.Screenshots / screen recording:
Test setup:
oc loginhttp://localhost:9000frontend/e2e/.envconfigured with cluster credentialsTest 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:
Additional info: