CONSOLE-5299: Migrate dev-console Cypress tests to Playwright (batch 1)#16712
CONSOLE-5299: Migrate dev-console Cypress tests to Playwright (batch 1)#16712shahsahil264 wants to merge 4 commits into
Conversation
Migrate dev-console Cypress E2E tests to Playwright as part of the broader Cypress→Playwright migration (CONSOLE-5239). This is batch 1 covering config maps, deployments, perspectives, customization, pod list, project creation, routes, and search. Tests migrated: 18 Cypress scenarios → 21 Playwright test cases Page objects created: ConfigMapPage, DeploymentPage, PodListPage, SearchPage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@shahsahil264: This pull request references CONSOLE-5299 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shahsahil264 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds dev-console Playwright page objects and end-to-end specs for ConfigMaps, Deployments, Pod lists, Search, Routes, project creation, perspective configuration, and a skipped Health Checks placeholder. ChangesDev Console E2E additions
Estimated code review effort: 2 (Simple) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 10❌ Failed checks (10 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (11)
frontend/e2e/tests/dev-console/config-maps.spec.ts (1)
49-54: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueValue textarea for the new key is never filled.
addKeyValue()adds a row and onlygetKeyInput(1)is filled with'key-test1'; the corresponding value textarea is left empty beforesave(). If the form requires both fields, this could be masking a validation gap or leaving the test only partially exercising the "add key-value" flow.✏️ Optional improvement to also fill and verify the value
await test.step('Add a new key-value pair and save', async () => { await configMapPage.addKeyValue(); await expect(configMapPage.getKeyInput(1)).toBeVisible(); await configMapPage.getKeyInput(1).fill('key-test1'); + await configMapPage.getValueTextarea(1).fill('value-test1'); await configMapPage.save(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/config-maps.spec.ts` around lines 49 - 54, The add-key-value test only fills the new key field and leaves the value textarea empty before saving, so the “add key-value” flow is not fully exercised. Update the dev-console config maps E2E step that calls addKeyValue() and getKeyInput(1) to also locate the matching value field for the new row, fill it with a test value, and optionally assert it is visible or populated before configMapPage.save().frontend/e2e/tests/dev-console/route.spec.ts (2)
7-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyfor fixture parameters.
k8sClient: anyandcleanup: anydiscard type safety for these fixtures. Both are already strongly typed in the../../fixturesmodule used elsewhere in this file (page, k8sClient, cleanupdestructured in thetest(...)callbacks) — reuse those types here instead.♻️ Proposed fix
+import type { KubernetesClient, TestCleanup } from '../../fixtures'; // adjust to actual exported type names + async function createRoutePrerequisites( - k8sClient: any, - cleanup: any, + k8sClient: KubernetesClient, + cleanup: TestCleanup, namespace: string, ): Promise<void> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/route.spec.ts` around lines 7 - 11, Replace the `any`-typed fixture parameters in `createRoutePrerequisites` with the same concrete types used by the `test(...)` fixtures from `../../fixtures`, so `k8sClient` and `cleanup` keep full type safety. Update the `createRoutePrerequisites` signature to reference those fixture types directly, and keep the rest of the helper logic unchanged.
66-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate route-creation body across both tests.
The
route.openshift.io/v1Route object is repeated verbatim in both tests. Extract it into a small helper (similar tocreateRoutePrerequisites) to avoid drift if the route shape needs to change later.♻️ Proposed fix
+ async function createTestRoute(k8sClient: any, namespace: string, routeName: string): Promise<void> { + await k8sClient.createCustomResource('route.openshift.io', 'v1', namespace, 'routes', { + apiVersion: 'route.openshift.io/v1', + kind: 'Route', + metadata: { name: routeName, namespace }, + spec: { + to: { kind: 'Service', name: serviceName }, + port: { targetPort: servicePort }, + }, + }); + }Then call
await createTestRoute(k8sClient, ns, routeName);in bothtest.stepblocks.Also applies to: 93-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/route.spec.ts` around lines 66 - 74, The Route object creation is duplicated in both tests, so extract the repeated `k8sClient.createCustomResource('route.openshift.io', 'v1', ...)` body into a small helper like `createTestRoute`, similar to `createRoutePrerequisites`. Keep the route manifest construction in that helper and replace both inline blocks with `await createTestRoute(k8sClient, ns, routeName);` so changes to the Route shape stay in one place.frontend/e2e/tests/dev-console/pod-list.spec.ts (1)
12-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate pod creation logic across both tests.
The pod spec (metadata, security context, container config) is copy-pasted verbatim between both tests, differing only by namespace. Consider extracting a shared helper to reduce duplication and ease future maintenance (e.g., if the security context requirements change again).
♻️ Suggested refactor
+async function createTestPod(k8sClient, ns: string) { + await k8sClient.createPod({ + apiVersion: 'v1', + kind: 'Pod', + metadata: { name: 'test-pod', namespace: ns }, + spec: { + securityContext: { + runAsNonRoot: true, + seccompProfile: { type: 'RuntimeDefault' }, + }, + containers: [ + { + name: 'test', + image: 'registry.access.redhat.com/ubi9/ubi-minimal:latest', + command: ['sleep', '3600'], + securityContext: { + allowPrivilegeEscalation: false, + capabilities: { drop: ['ALL'] }, + }, + }, + ], + }, + }); +}Then call
await createTestPod(k8sClient, ns);in bothtest.stepblocks.Also applies to: 50-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/pod-list.spec.ts` around lines 12 - 33, The pod creation setup in the two pod-list tests is duplicated, with only the namespace changing. Extract the shared pod spec into a helper such as createTestPod that accepts k8sClient and ns, and move the repeated metadata, securityContext, and container config into it. Then replace both inline createPod calls in the pod-list spec with await createTestPod(k8sClient, ns) inside the test.step blocks.frontend/e2e/tests/dev-console/project-creation.spec.ts (1)
25-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen success assertion for project creation.
Only checking that the "Create Project" heading is detached doesn't confirm the project was actually created — it would also pass if the modal closed due to an error/cancel path. Consider asserting on a stronger success signal (e.g., URL navigation to the new project/topology view, or verifying the project name appears in the namespace dropdown/list).
✅ Suggested stronger assertion
await test.step('Verify modal closed', async () => { await expect(page.locator('h1', { hasText: 'Create Project' })).not.toBeAttached({ timeout: 30_000, }); + // Confirm the project was actually created, e.g. redirected into its namespace + await expect(page).toHaveURL(new RegExp(projectName)); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/project-creation.spec.ts` around lines 25 - 29, The current success check in project creation only verifies the "Create Project" modal closes, which is not strong enough to prove the project was created. Update the assertion in the project creation test to use a real success signal from the flow, such as confirming navigation to the new project/topology view or checking the created project name appears in the namespace dropdown/list. Use the existing test.step and page.locator logic in project-creation.spec.ts to replace the modal-closed check with a stronger post-submit verification.frontend/e2e/tests/dev-console/customization-catalog-form.spec.ts (2)
12-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated navigation/setup across tests.
All three tests repeat
page.goto(...)+page.getByTestId('Customize').click(). Extracting this into atest.beforeEachwould reduce duplication while keeping tests independent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/customization-catalog-form.spec.ts` around lines 12 - 29, The three Playwright tests repeat the same navigation and setup sequence, so move the shared `page.goto('/k8s/cluster/operator.openshift.io~v1~Console/cluster')` and `page.getByTestId('Customize').click()` steps into a `test.beforeEach` in `customization-catalog-form.spec.ts`. Keep the individual assertions and tab-specific clicks inside `verifies perspectives section on General tab` and `verifies Developer tab shows pre-pinned navigation` so each test remains independent while removing duplication.
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a role-based tab locator
.pf-v6-c-tabs__itemcouples this test to PatternFly markup;page.getByRole('tab', { name: 'Developer' })is more resilient and matches the rest of the E2E suite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/customization-catalog-form.spec.ts` at line 25, The tab selection in the customization catalog form test is too tightly coupled to PatternFly CSS markup. Update the click in the test that uses page.locator('.pf-v6-c-tabs__item', { hasText: 'Developer' }) to use the role-based tab locator instead, matching the rest of the E2E suite and keeping the test resilient. Keep the change focused on the Developer tab interaction in the relevant test spec.frontend/e2e/tests/dev-console/configure-perspectives.spec.ts (1)
15-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate coverage with
customization-catalog-form.spec.ts.This test's steps (navigate to cluster config, click Customize, assert "Cluster configuration" heading) are identical to
verifies cluster configuration page loadsincustomization-catalog-form.spec.ts(lines 4-10). Consider removing one, since both files exercise the same page-load path and any change to the "Customize" flow would need updating in two places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/configure-perspectives.spec.ts` around lines 15 - 22, The cluster configuration navigation test duplicates the same page-load coverage already exercised by the corresponding test in customization-catalog-form.spec.ts. Remove one of the redundant cases and keep a single source of truth for the Customize flow, using the existing test names and the shared “Cluster configuration” heading assertion to locate the duplicated coverage.frontend/e2e/pages/dev-console/search-page.ts (2)
29-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnscoped text locators risk matching unrelated elements.
getRecentlyUsedHeading()andgetRecentlyUsedItem()search the whole page for exact text matches. Resource kind names (e.g., "ConfigMap", "Deployment") can also appear elsewhere on the page (breadcrumbs, nav labels, resource icons), so.first()could silently match the wrong element instead of failing loudly. Consider scoping these locators to the select/dropdown container.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/pages/dev-console/search-page.ts` around lines 29 - 35, The text locators in SearchPage are too broad and can match unrelated page content; scope both getRecentlyUsedHeading() and getRecentlyUsedItem() to the recently used select/dropdown container instead of searching the full page. Update the locators in SearchPage so they resolve within the specific UI section that renders the recently used list, and avoid relying on .first() to mask ambiguous matches.
24-27: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnscoped role-based locator for the close/clear-history button is fragile.
getByRole('button', { name: 'Close' })performs a case-insensitive substring match against accessible name and isn't scoped to the search dropdown. If any other "Close"-named control is rendered on the page at the same time (toast, modal, notification drawer), this will hit a Playwright strict-mode violation or click the wrong element. The underlying component already exposes a stable test id (dataTestID="close-icon") perresource-dropdown.tsx, which would be more robust than accessible-name matching.♻️ Suggested fix using the existing test id
- async clearHistory(): Promise<void> { - const closeButton = this.page.getByRole('button', { name: 'Close' }); - await this.robustClick(closeButton); - } + async clearHistory(): Promise<void> { + const closeButton = this.page.locator('[data-test="close-icon"], [data-test-id="close-icon"]'); + await this.robustClick(closeButton); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/pages/dev-console/search-page.ts` around lines 24 - 27, The clear-history action in `SearchPage.clearHistory()` uses an unscoped role/name locator that can match the wrong Close control. Update the locator to use the stable test id exposed by the dropdown component (`dataTestID="close-icon"`) instead of `getByRole('button', { name: 'Close' })`, and keep the click routed through `robustClick` so the selector remains reliable even when other Close buttons are present.frontend/e2e/tests/dev-console/search.spec.ts (1)
30-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest doesn't verify the "cap at 5" boundary.
The title "shows up to 5 recently searched items" implies a cap, but the test only confirms 5 items appear after searching exactly 5 — it doesn't verify that a 6th search evicts the oldest entry. Consider adding a 6th resource search and asserting the first one is no longer present to actually cover the boundary behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/dev-console/search.spec.ts` around lines 30 - 63, The test currently only proves that five recently searched items can appear, but it does not verify the cap behavior. Update the SearchPage-driven flow in shows up to 5 recently searched items by adding a 6th resource search after the existing resources loop, then assert that the oldest entry is evicted and no longer visible in the recently used list when reopening the resources filter. Keep the check centered on the existing SearchPage helpers and getRecentlyUsedItem so the boundary behavior is exercised directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/e2e/pages/dev-console/search-page.ts`:
- Around line 29-35: The text locators in SearchPage are too broad and can match
unrelated page content; scope both getRecentlyUsedHeading() and
getRecentlyUsedItem() to the recently used select/dropdown container instead of
searching the full page. Update the locators in SearchPage so they resolve
within the specific UI section that renders the recently used list, and avoid
relying on .first() to mask ambiguous matches.
- Around line 24-27: The clear-history action in `SearchPage.clearHistory()`
uses an unscoped role/name locator that can match the wrong Close control.
Update the locator to use the stable test id exposed by the dropdown component
(`dataTestID="close-icon"`) instead of `getByRole('button', { name: 'Close' })`,
and keep the click routed through `robustClick` so the selector remains reliable
even when other Close buttons are present.
In `@frontend/e2e/tests/dev-console/config-maps.spec.ts`:
- Around line 49-54: The add-key-value test only fills the new key field and
leaves the value textarea empty before saving, so the “add key-value” flow is
not fully exercised. Update the dev-console config maps E2E step that calls
addKeyValue() and getKeyInput(1) to also locate the matching value field for the
new row, fill it with a test value, and optionally assert it is visible or
populated before configMapPage.save().
In `@frontend/e2e/tests/dev-console/configure-perspectives.spec.ts`:
- Around line 15-22: The cluster configuration navigation test duplicates the
same page-load coverage already exercised by the corresponding test in
customization-catalog-form.spec.ts. Remove one of the redundant cases and keep a
single source of truth for the Customize flow, using the existing test names and
the shared “Cluster configuration” heading assertion to locate the duplicated
coverage.
In `@frontend/e2e/tests/dev-console/customization-catalog-form.spec.ts`:
- Around line 12-29: The three Playwright tests repeat the same navigation and
setup sequence, so move the shared
`page.goto('/k8s/cluster/operator.openshift.io~v1~Console/cluster')` and
`page.getByTestId('Customize').click()` steps into a `test.beforeEach` in
`customization-catalog-form.spec.ts`. Keep the individual assertions and
tab-specific clicks inside `verifies perspectives section on General tab` and
`verifies Developer tab shows pre-pinned navigation` so each test remains
independent while removing duplication.
- Line 25: The tab selection in the customization catalog form test is too
tightly coupled to PatternFly CSS markup. Update the click in the test that uses
page.locator('.pf-v6-c-tabs__item', { hasText: 'Developer' }) to use the
role-based tab locator instead, matching the rest of the E2E suite and keeping
the test resilient. Keep the change focused on the Developer tab interaction in
the relevant test spec.
In `@frontend/e2e/tests/dev-console/pod-list.spec.ts`:
- Around line 12-33: The pod creation setup in the two pod-list tests is
duplicated, with only the namespace changing. Extract the shared pod spec into a
helper such as createTestPod that accepts k8sClient and ns, and move the
repeated metadata, securityContext, and container config into it. Then replace
both inline createPod calls in the pod-list spec with await
createTestPod(k8sClient, ns) inside the test.step blocks.
In `@frontend/e2e/tests/dev-console/project-creation.spec.ts`:
- Around line 25-29: The current success check in project creation only verifies
the "Create Project" modal closes, which is not strong enough to prove the
project was created. Update the assertion in the project creation test to use a
real success signal from the flow, such as confirming navigation to the new
project/topology view or checking the created project name appears in the
namespace dropdown/list. Use the existing test.step and page.locator logic in
project-creation.spec.ts to replace the modal-closed check with a stronger
post-submit verification.
In `@frontend/e2e/tests/dev-console/route.spec.ts`:
- Around line 7-11: Replace the `any`-typed fixture parameters in
`createRoutePrerequisites` with the same concrete types used by the `test(...)`
fixtures from `../../fixtures`, so `k8sClient` and `cleanup` keep full type
safety. Update the `createRoutePrerequisites` signature to reference those
fixture types directly, and keep the rest of the helper logic unchanged.
- Around line 66-74: The Route object creation is duplicated in both tests, so
extract the repeated `k8sClient.createCustomResource('route.openshift.io', 'v1',
...)` body into a small helper like `createTestRoute`, similar to
`createRoutePrerequisites`. Keep the route manifest construction in that helper
and replace both inline blocks with `await createTestRoute(k8sClient, ns,
routeName);` so changes to the Route shape stay in one place.
In `@frontend/e2e/tests/dev-console/search.spec.ts`:
- Around line 30-63: The test currently only proves that five recently searched
items can appear, but it does not verify the cap behavior. Update the
SearchPage-driven flow in shows up to 5 recently searched items by adding a 6th
resource search after the existing resources loop, then assert that the oldest
entry is evicted and no longer visible in the recently used list when reopening
the resources filter. Keep the check centered on the existing SearchPage helpers
and getRecentlyUsedItem so the boundary behavior is exercised directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d350d320-cbff-4e0f-8a13-4d44091ccd8a
📒 Files selected for processing (13)
frontend/e2e/pages/dev-console/config-map-page.tsfrontend/e2e/pages/dev-console/deployment-page.tsfrontend/e2e/pages/dev-console/pod-list-page.tsfrontend/e2e/pages/dev-console/search-page.tsfrontend/e2e/tests/dev-console/config-maps.spec.tsfrontend/e2e/tests/dev-console/configure-perspectives.spec.tsfrontend/e2e/tests/dev-console/customization-catalog-form.spec.tsfrontend/e2e/tests/dev-console/deployment.spec.tsfrontend/e2e/tests/dev-console/health-checks.spec.tsfrontend/e2e/tests/dev-console/pod-list.spec.tsfrontend/e2e/tests/dev-console/project-creation.spec.tsfrontend/e2e/tests/dev-console/route.spec.tsfrontend/e2e/tests/dev-console/search.spec.ts
- Scope search locators to resource dropdown list container
- Use data-test-id="close-icon" for clear history button
- Fill value field when adding key-value pair in ConfigMap edit test
- Remove duplicate cluster config navigation test from configure-perspectives
- Extract beforeEach for shared setup in customization tests
- Use getByRole('tab') instead of PF CSS class for Developer tab
- Extract createTestPod helper to deduplicate pod-list tests
- Stronger project creation assertion (verify namespace dropdown contains project)
- Type route helper params with KubernetesClient and CleanupFixture
- Extract createTestRoute helper to deduplicate route tests
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@shahsahil264: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
ConfigMap and Deployment forms use a SyncedEditorField that toggles between Form and YAML views based on user preferences. On CI with a fresh user, the editor may default to YAML view. Add ensureFormView() that waits for the synced editor toggle to render, switches to Form view if needed, and waits for the name input to be visible. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/retest |
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Analysis / Root cause:
Migrate dev-console Cypress E2E tests to Playwright as part of the broader Cypress→Playwright migration (CONSOLE-5239). Batch 1 covers config maps, deployments, perspectives, cluster configuration customization, pod list, project creation, routes, and search.
Solution description:
e2e/pages/dev-console/:ConfigMapPage,DeploymentPage,PodListPage,SearchPagee2e/tests/dev-console/covering:DetailsPage,ListPage) where applicableTests migrated: 18 Cypress scenarios → 21 Playwright test cases
Screenshots / screen recording:
N/A — test infrastructure only, no UI changes
Test setup:
e2e/.envwith cluster credentialsyarn playwright test --project=dev-consoleTest cases:
npx tsc --noEmit -p e2e/tsconfig.json)Browser conformance:
Additional info:
This is batch 1 of the dev-console migration. Remaining feature files will be migrated in subsequent batches.
Reviewers and assignees:
🤖 Generated with Claude Code
Summary by CodeRabbit