CONSOLE-5235: Migrate basic app Cypress e2e tests to Playwright#16431
CONSOLE-5235: Migrate basic app Cypress e2e tests to Playwright#16431stefanonardo wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@stefanonardo: This pull request references CONSOLE-5235 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. |
openshift-ci
Bot
added
the
kind/cypress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: stefanonardo 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 |
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThis pull request establishes the Cypress-to-Playwright migration framework for Console e2e tests. It adds migration documentation ( Suggested reviewers
🚥 Pre-merge checks | ✅ 8 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
frontend/e2e/pages/catalog-page.ts (1)
6-8: ⚡ Quick winUse the stable
data-testattribute for the keyword filter input.The underlying SearchInput component (CatalogToolbar.tsx) provides
data-test="search-catalog". Update the selector from'input[placeholder*="Filter by keyword"]'tothis.page.getByTestId('search-catalog')to avoid brittleness from placeholder text changes and i18n updates. Aligns with the existing pattern used incatalogItem(testId)on line 19.🤖 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/catalog-page.ts` around lines 6 - 8, Replace the brittle placeholder-based locator for the keyword filter by updating the private field filterInput (type Locator) to use the stable test id selector: use this.page.getByTestId('search-catalog') instead of 'input[placeholder*="Filter by keyword"]' so the CatalogToolbar's data-test="search-catalog" is targeted like catalogItem(testId) does.frontend/e2e/tests/console/app/masthead.spec.ts (1)
58-67: ⚡ Quick winAdd a post-logout assertion to avoid false positives.
The test currently validates only clicks. Please assert an observable logout outcome (URL/login screen/auth redirect) after Line 67.
🤖 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/app/masthead.spec.ts` around lines 58 - 67, The test only performs clicks and can falsely pass; after calling MastheadPage.clickLogOut() add a concrete post-logout assertion: if using URL-based redirect, await page.waitForURL(...) or expect(page).toHaveURL(...) for the login or landing path; alternatively assert a login-related UI element via MastheadPage (e.g., expect(masthead.loginButton).toBeVisible() or expect(masthead.isAuthenticated()).resolves.toBe(false)). Use the existing MastheadPage helpers (isAuthDisabled, openUserDropdown, clickLogOut) and add the appropriate await/expect to verify the app reached the logged-out state.frontend/e2e/tests/console/app/filtering-and-searching.spec.ts (1)
27-55: ⚡ Quick winDrop
as anyfor the deployment body.Line 54 removes compile-time guarantees for your setup manifest. Prefer a typed value using
satisfiesagainstcreateDeploymentinput.Suggested fix
- await client.createDeployment(ns, { + const deployment = { apiVersion: 'apps/v1', kind: 'Deployment', metadata: { name: workloadName, labels: { 'lbl-filter': ns, app: 'name' }, }, spec: { replicas: 3, selector: { matchLabels: { app: 'name' } }, template: { metadata: { labels: { app: 'name' } }, spec: { securityContext: { runAsNonRoot: true, seccompProfile: { type: 'RuntimeDefault' } }, containers: [ { name: 'httpd', image: 'image-registry.openshift-image-registry.svc:5000/openshift/httpd:latest', securityContext: { allowPrivilegeEscalation: false, capabilities: { drop: ['ALL'] }, }, }, ], }, }, }, - } as any); + } satisfies Parameters<KubernetesClient['createDeployment']>[1]; + await client.createDeployment(ns, deployment);As per coding guidelines: “Avoid using
anytype; flag use ofanytype and suggest proper type definitions”.🤖 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/app/filtering-and-searching.spec.ts` around lines 27 - 55, Remove the unsafe "as any" cast on the deployment manifest and instead satisfy the actual parameter type expected by client.createDeployment; replace the trailing "as any" with "satisfies Parameters<typeof client.createDeployment>[1]" (or the concrete input type for createDeployment) so the manifest literal (the object passed to client.createDeployment in the test) is type-checked, keeping the rest of the call to client.waitForDeploymentReady(workloadName, ns) unchanged.frontend/e2e/pages/masthead-page.ts (1)
33-35: ⚡ Quick winReplace
anyinisAuthDisabledwith a narrow window type.Line 34 can be fully typed without
any, keeping strict checks intact.Suggested fix
async isAuthDisabled(): Promise<boolean> { - return this.page.evaluate(() => !!(window as any).SERVER_FLAGS?.authDisabled); + return this.page.evaluate(() => { + const flags = (window as Window & { SERVER_FLAGS?: { authDisabled?: boolean } }).SERVER_FLAGS; + return Boolean(flags?.authDisabled); + }); }As per coding guidelines: “Avoid using
anytype; flag use ofanytype and suggest proper type definitions”.🤖 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/masthead-page.ts` around lines 33 - 35, The isAuthDisabled method uses (window as any) which bypasses typings; replace it with a narrow Window interface that includes optional SERVER_FLAGS with authDisabled and use that type in the page.evaluate callback. Add a local type or interface like interface WindowWithServerFlags { SERVER_FLAGS?: { authDisabled?: boolean } } and then change the evaluate body to cast window to WindowWithServerFlags (or declare it in the callback signature) so the expression becomes !!(windowAsTyped.SERVER_FLAGS?.authDisabled) while removing any usage; update the function isAuthDisabled to use that typed window reference.frontend/e2e/pages/logs-page.ts (1)
16-18: ⚡ Quick winUse stable test IDs instead of placeholder/class-based locators
Line 16-18 rely on a localized placeholder and CSS classes, which are brittle for long-term e2e stability. Prefer
data-test-backed locators for these elements as well.As per coding guidelines:
**/*.{tsx,ts}: Preferdata-testattributes for Cypress selectors ... over brittle CSS or ARIA selectors.🤖 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/logs-page.ts` around lines 16 - 18, Replace brittle locators in logs-page.ts with data-test-backed selectors: update the Locator definitions searchInput, searchMatches, and logText to use stable data-test attributes (e.g., page.locator('[data-test="logs-search"]'), page.locator('[data-test="logs-match"]'), page.locator('[data-test="log-text"]')) instead of the placeholder or class-based selectors, and ensure the corresponding components/templates include those data-test attributes so the tests can target them reliably.frontend/e2e/tests/console/app/resource-log.spec.ts (1)
92-92: ⚡ Quick winDrop
as anyfor Pod specs to preserve type safetyLine 92, 126, and 127 cast Pod specs to
any, which hides schema mistakes in test fixtures. Please use a concrete k8s resource type for these objects.As per coding guidelines:
**/*.{ts,tsx}: Avoid usinganytype; flag use ofanytype and suggest proper type definitions.Also applies to: 126-127
🤖 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/app/resource-log.spec.ts` at line 92, The test is casting Pod fixtures to any (examplePodSpec) before calling k8sClient.createPod which loses type safety; replace the `as any` casts by typing the fixture objects with the concrete Kubernetes Pod type used by your k8s client (e.g., V1Pod or the client's PodManifest type), import that type at the top, update examplePodSpec (and the other pod spec variables used around createPod) to match that interface, and pass them directly to k8sClient.createPod so the compiler validates the Pod schema instead of using `any`.
🤖 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 @.claude/skills/migrate-cypress/SKILL.md:
- Around line 79-81: Update the selector examples to match the Selector
Unification rule by replacing occurrences of the legacy attribute `data-test-id`
with the unified `data-test` in the SKILL.md examples; specifically change the
locator initialization referenced by the symbol detailsTab (the
this.page.locator('[data-test-id="horizontal-link-Details"]')) and the other
example block around the second occurrence (lines shown in the review) so both
examples use '[data-test="horizontal-link-Details"]' and analogous selectors to
keep the documentation consistent.
- Around line 24-28: Update the fenced code blocks in SKILL.md so they declare a
language (e.g., add "text" after the opening ```), specifically for the
migration command block containing the lines starting with "/migrate-cypress
..." and the corresponding output block showing "Migration complete:
<source-file> → <output-file>" (also apply the same change to the second
occurrence around lines 106-113); open each triple-backtick fence and change ```
to ```text so markdownlint MD040 warnings are resolved.
In `@frontend/e2e/clients/kubernetes-client.ts`:
- Around line 489-495: The waitForPodReady function currently only checks
pod.status.phase === 'Running' and can return before containers are actually
ready; update the poll predicate in waitForPodReady to fetch the pod via
this.k8sApi.readNamespacedPod and then verify readiness by (a) confirming
status.phase === 'Running' and (b) ensuring either all status.containerStatuses
exist and every containerStatus.ready === true OR that pod.status.conditions
contains a condition with type === 'Ready' and status === 'True'; handle missing
fields defensively (treat absent containerStatuses/conditions as not ready) and
keep the existing timeout/polling behavior so callers of waitForPodReady get a
true only when the pod is actually ready.
In `@frontend/e2e/pages/list-page.ts`:
- Around line 54-60: The clickFirstRowLinkMatching method can misbehave if
callers pass a RegExp with global or sticky flags because RegExp.lastIndex is
stateful; defensively create a fresh RegExp without the g or y flags from the
incoming pattern before the loop (use pattern.source and pattern.flags filtered
to remove 'g' and 'y') and then use that new RegExp for matching inside the
loop, keeping the rest of the logic (including robustClick) unchanged.
In `@frontend/e2e/tests/console/app/resource-log.spec.ts`:
- Line 65: The test uses absolute paths in page.goto (e.g.
page.goto('/k8s/ns/openshift-kube-apiserver/core~v1~Pod')) which breaks behind
non-root proxy; replace these calls with the base-path-safe routing helper used
elsewhere (for example a getConsoleRoute or buildConsoleUrl helper) so the path
is prefixed by the app's base path, and update each occurrence of
page.goto('/k8s/...') in resource-log.spec.ts to call that helper (e.g. await
page.goto(getConsoleRoute('k8s/ns/openshift-kube-apiserver/core~v1~Pod'))) so
tests work when Console is hosted under a non-root path.
In `@frontend/e2e/tests/console/app/template.spec.ts`:
- Line 4: The test uses a fixed constant TEMPLATE_NAME ('httpd-example-test')
causing collisions; update the spec (template.spec.ts) to generate a unique
template name at runtime (e.g., append a timestamp or UUID) and replace the
constant TEMPLATE_NAME with that dynamic value so each run uses a distinct name;
ensure the generated name is used wherever TEMPLATE_NAME is referenced in the
test so creation calls no longer clash with existing resources.
---
Nitpick comments:
In `@frontend/e2e/pages/catalog-page.ts`:
- Around line 6-8: Replace the brittle placeholder-based locator for the keyword
filter by updating the private field filterInput (type Locator) to use the
stable test id selector: use this.page.getByTestId('search-catalog') instead of
'input[placeholder*="Filter by keyword"]' so the CatalogToolbar's
data-test="search-catalog" is targeted like catalogItem(testId) does.
In `@frontend/e2e/pages/logs-page.ts`:
- Around line 16-18: Replace brittle locators in logs-page.ts with
data-test-backed selectors: update the Locator definitions searchInput,
searchMatches, and logText to use stable data-test attributes (e.g.,
page.locator('[data-test="logs-search"]'),
page.locator('[data-test="logs-match"]'),
page.locator('[data-test="log-text"]')) instead of the placeholder or
class-based selectors, and ensure the corresponding components/templates include
those data-test attributes so the tests can target them reliably.
In `@frontend/e2e/pages/masthead-page.ts`:
- Around line 33-35: The isAuthDisabled method uses (window as any) which
bypasses typings; replace it with a narrow Window interface that includes
optional SERVER_FLAGS with authDisabled and use that type in the page.evaluate
callback. Add a local type or interface like interface WindowWithServerFlags {
SERVER_FLAGS?: { authDisabled?: boolean } } and then change the evaluate body to
cast window to WindowWithServerFlags (or declare it in the callback signature)
so the expression becomes !!(windowAsTyped.SERVER_FLAGS?.authDisabled) while
removing any usage; update the function isAuthDisabled to use that typed window
reference.
In `@frontend/e2e/tests/console/app/filtering-and-searching.spec.ts`:
- Around line 27-55: Remove the unsafe "as any" cast on the deployment manifest
and instead satisfy the actual parameter type expected by
client.createDeployment; replace the trailing "as any" with "satisfies
Parameters<typeof client.createDeployment>[1]" (or the concrete input type for
createDeployment) so the manifest literal (the object passed to
client.createDeployment in the test) is type-checked, keeping the rest of the
call to client.waitForDeploymentReady(workloadName, ns) unchanged.
In `@frontend/e2e/tests/console/app/masthead.spec.ts`:
- Around line 58-67: The test only performs clicks and can falsely pass; after
calling MastheadPage.clickLogOut() add a concrete post-logout assertion: if
using URL-based redirect, await page.waitForURL(...) or
expect(page).toHaveURL(...) for the login or landing path; alternatively assert
a login-related UI element via MastheadPage (e.g.,
expect(masthead.loginButton).toBeVisible() or
expect(masthead.isAuthenticated()).resolves.toBe(false)). Use the existing
MastheadPage helpers (isAuthDisabled, openUserDropdown, clickLogOut) and add the
appropriate await/expect to verify the app reached the logged-out state.
In `@frontend/e2e/tests/console/app/resource-log.spec.ts`:
- Line 92: The test is casting Pod fixtures to any (examplePodSpec) before
calling k8sClient.createPod which loses type safety; replace the `as any` casts
by typing the fixture objects with the concrete Kubernetes Pod type used by your
k8s client (e.g., V1Pod or the client's PodManifest type), import that type at
the top, update examplePodSpec (and the other pod spec variables used around
createPod) to match that interface, and pass them directly to
k8sClient.createPod so the compiler validates the Pod schema instead of using
`any`.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 863a79dc-002d-49da-ba01-806f799f3415
📒 Files selected for processing (30)
.claude/migration-context.md.claude/skills/debug-test/SKILL.md.claude/skills/migrate-cypress/SKILL.md.gitignoreAGENTS.mdfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/packages/integration-tests/fixtures/httpd-example-template.yamlfrontend/packages/integration-tests/fixtures/pod-with-space.yamlfrontend/packages/integration-tests/fixtures/pod-with-wrap-annotation.yamlfrontend/packages/integration-tests/tests/app/filtering-and-searching.cy.tsfrontend/packages/integration-tests/tests/app/masthead.cy.tsfrontend/packages/integration-tests/tests/app/node-terminal.cy.tsfrontend/packages/integration-tests/tests/app/overview.cy.tsfrontend/packages/integration-tests/tests/app/resource-log.cy.tsfrontend/packages/integration-tests/tests/app/template.cy.tsfrontend/packages/integration-tests/views/catalogs.tsfrontend/packages/integration-tests/views/logs.tsfrontend/packages/integration-tests/views/overview.ts
💤 Files with no reviewable changes (12)
- frontend/packages/integration-tests/tests/app/resource-log.cy.ts
- frontend/packages/integration-tests/fixtures/pod-with-space.yaml
- frontend/packages/integration-tests/fixtures/pod-with-wrap-annotation.yaml
- frontend/packages/integration-tests/tests/app/overview.cy.ts
- frontend/packages/integration-tests/views/overview.ts
- frontend/packages/integration-tests/tests/app/node-terminal.cy.ts
- frontend/packages/integration-tests/tests/app/template.cy.ts
- frontend/packages/integration-tests/fixtures/httpd-example-template.yaml
- frontend/packages/integration-tests/tests/app/filtering-and-searching.cy.ts
- frontend/packages/integration-tests/views/catalogs.ts
- frontend/packages/integration-tests/views/logs.ts
- frontend/packages/integration-tests/tests/app/masthead.cy.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{tsx,ts}
📄 CodeRabbit inference engine (TESTING.md)
Prefer
data-testattributes for Cypress selectors (e.g.,cy.get('[data-test="create-deployment"]')) over brittle CSS or ARIA selectors
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Use lowercase dash-separated names for all files (to avoid git issues with case-insensitive file systems)
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
**/*.{ts,tsx}: Prefer functional programming patterns and immutable data structures
Run the linter and follow all rules defined in .eslintrc
Use React hooks and Context API for state management, migrating away from legacy Redux/Immutable.js
Use existing hooks fromconsole-sharedwhen possible (useK8sWatchResource,useUserSettings, etc.)
Use k8s resource hooks for data fetching,consoleFetchJSONfor HTTP requests
Use console extension points for plugin integration
Check existing types inconsole-sharedbefore creating new types
UseuseTranslation('namespace')hook withkeyformat for translation keys
UseuseCallbackto memoize callbacks and prevent unnecessary re-renders
UseuseMemofor expensive filtering and computations to prevent re-renders
Avoid usinganytype; flag use ofanytype and suggest proper type definitions
Verify null/undefined are properly handled in type definitions (usestring | undefinedformat)
Verify exported types for reusable components are properly defined
UseusePluginInfohook for plugin data access
Avoid importing from deprecated component paths (check for/deprecatedin import paths and@deprecatedJSDoc tags)
Use direct imports to specific files instead of barrel exports from index.ts to avoid circular dependencies and improve build performance
Useimport typefor type-only imports to improve tree-shaking and build performance
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Never use absolute paths in code; the app must be able to run behind a proxy under an arbitrary path
**/*.{ts,tsx,js,jsx}: Use i18next'sTFunctioninside functions or components, not in module scope
Don't use backticks inside of aTFunctioncall; use single quotes instead for template strings
For dynamic keys that cannot be interpolated by i18next-parser, specify possible static values in comments (e.g.,// t('key_1')) to aid key generation
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Tests should follow a table-driven tests convention similar to Go where applicable
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*.ts
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Updates to
console-dynamic-plugin-sdkshould maintain backward compatibility as it's a public API; use theplugin-api-reviewskill to vet changes for public API impact
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Use lowercase dash-separated names for all files and directories, with exceptions for files with their own naming conventions (Dockerfile, Makefile, README, etc.)
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsAGENTS.mdfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (INTERNATIONALIZATION.md)
**/*.{ts,tsx,jsx}: Internationalizearia-label,aria-placeholder,aria-roledescription, andaria-valuetextattributes
When displaying a resource kind, use the predefinedmodel.labelPluralKeywrapped inTFunctionif available, otherwise fall back tomodel.labelPlural
Use the i18nKey property on react-i18next Trans component only as a last resort when the parser generates incorrect keys with HTML tags
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (AGENTS.md)
Never import from package index files (e.g.,
@console/shared) in new code; import from specific file paths instead to avoid circular dependencies and slow buildsEnsure all
$codeRefin extension points ALWAYS reference the corresponding extension type from the dynamic plugin SDK package (@console/dynamic-plugin-sdk/src/extensions/) for type safetyNever use template literals (backticks) in
t()i18n calls; use single or double quotes instead as the i18n parser cannot extract keys from template literalsNever import from or use code with the
@deprecatedTSdoc tag in new codeNever use absolute URLs or paths; the console runs behind a proxy under an arbitrary path
After adding or modifying user-facing strings, run
yarn i18nto update i18n keys and commit updated keys alongside code changes that affect i18n
Files:
frontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/list-page.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.ts
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent capabilities and behavior in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: Before starting ANY changes to the dynamic plugin SDK, ensure changes do not impact the public API by checking `frontend/packages/console-dynamic-plugin-sdk/src/api/internal-*.ts` files to avoid breakage of external plugins
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: Always consider impact on external plugin developers when making changes to the dynamic plugin SDK, maintain backward compatibility, provide comprehensive documentation for all public APIs, and ensure changes to extension schemas include migration paths
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: Use the `plugin-api-review` skill for all changes to the dynamic plugin SDK public API to ensure proper vetting and prevent breaking changes
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: Backend dependency updates should be in separate commits from core logic changes to isolate vendor folder changes
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: Bug fix commits should be prefixed with bug number or Jira key (e.g., `OCPBUGS-1234: Fix ...`); commit subject line answers 'what changed' and body answers 'why'
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: When opening a PR, fill out the PR template located in `docs/pull_request_template.md` with all required sections and always link to the relevant JIRA issue in the PR title and description
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: Feature work branches should be named with `CONSOLE-####` (Jira story number); bug fix branches should be named with `OCPBUGS-####` (Jira bug number); use `main` as base branch
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-05-12T07:24:22.768Z
Learning: Use `/migrate-cypress` to convert Cypress e2e tests to Playwright and `/debug-test` to fix failing tests during the Cypress to Playwright migration
🪛 LanguageTool
.claude/skills/debug-test/SKILL.md
[style] ~100-~100: The adverb ‘sometimes’ is usually put before the verb ‘passes’.
Context: ...structure setup first. ### Flaky test (passes sometimes, fails sometimes) If a test passes on r...
(ADVERB_WORD_ORDER)
[style] ~100-~100: The adverb ‘sometimes’ is usually put before the verb ‘fails’.
Context: ...rst. ### Flaky test (passes sometimes, fails sometimes) If a test passes on re-run without any...
(ADVERB_WORD_ORDER)
🪛 markdownlint-cli2 (0.22.1)
.claude/skills/migrate-cypress/SKILL.md
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 106-106: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
frontend/e2e/tests/console/app/resource-log.spec.ts (1)
58-157: Solid migration test flow and cleanup disciplineGood use of
test.step, namespace isolation, readiness waits, and cleanup tracking. This is a strong Playwright translation pattern for cluster-backed e2e coverage.
| async waitForPodReady(name: string, namespace: string, timeoutMs = 120_000): Promise<boolean> { | ||
| return pollUntil( | ||
| async () => { | ||
| try { | ||
| const pod = await this.k8sApi.readNamespacedPod({ name, namespace }); | ||
| return pod?.status?.phase === 'Running'; | ||
| } catch { |
There was a problem hiding this comment.
waitForPodReady can return before the pod is actually ready.
Line 494 checks only phase === 'Running'; pods can be Running while containers are still unready, which can race downstream assertions.
Proposed fix
async waitForPodReady(name: string, namespace: string, timeoutMs = 120_000): Promise<boolean> {
return pollUntil(
async () => {
try {
const pod = await this.k8sApi.readNamespacedPod({ name, namespace });
- return pod?.status?.phase === 'Running';
+ const isRunning = pod?.status?.phase === 'Running';
+ const readyCondition =
+ pod?.status?.conditions?.find((c) => c.type === 'Ready')?.status === 'True';
+ const containersReady = (pod?.status?.containerStatuses ?? []).every((c) => c.ready);
+ return isRunning && readyCondition && containersReady;
} catch {
return false;
}
},
timeoutMs,
2_000,
);
}#!/bin/bash
# Inspect readiness implementation and where it is used.
rg -n -C3 --type=ts '\bwaitForPodReady\s*\(' frontend/e2e
rg -n -C4 --type=ts "status\?\.phase === 'Running'" frontend/e2e/clients/kubernetes-client.ts🤖 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/clients/kubernetes-client.ts` around lines 489 - 495, The
waitForPodReady function currently only checks pod.status.phase === 'Running'
and can return before containers are actually ready; update the poll predicate
in waitForPodReady to fetch the pod via this.k8sApi.readNamespacedPod and then
verify readiness by (a) confirming status.phase === 'Running' and (b) ensuring
either all status.containerStatuses exist and every containerStatus.ready ===
true OR that pod.status.conditions contains a condition with type === 'Ready'
and status === 'True'; handle missing fields defensively (treat absent
containerStatuses/conditions as not ready) and keep the existing timeout/polling
behavior so callers of waitForPodReady get a true only when the pod is actually
ready.
| async clickFirstRowLinkMatching(pattern: RegExp): Promise<void> { | ||
| const links = this.dataViewCells.locator('a'); | ||
| const count = await links.count(); | ||
| for (let i = 0; i < count; i++) { | ||
| const text = await links.nth(i).textContent(); | ||
| if (text && pattern.test(text)) { | ||
| await this.robustClick(links.nth(i)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the exact content of the file at the mentioned lines
head -70 frontend/e2e/pages/list-page.ts | tail -25Repository: openshift/console
Length of output: 989
🏁 Script executed:
#!/bin/bash
# Search for the method in the file and surrounding context
rg -A 15 "clickFirstRowLinkMatching" frontend/e2e/pages/list-page.tsRepository: openshift/console
Length of output: 494
🏁 Script executed:
#!/bin/bash
# Search for usage of clickFirstRowLinkMatching to see how it's called
rg "clickFirstRowLinkMatching" --type tsRepository: openshift/console
Length of output: 292
🏁 Script executed (no clone):
#!/bin/bash
# Verify RegExp.test() stateful behavior with g flag
node <<'EOF'
// Test 1: Regex with g flag - shows the stateful behavior
const patternWithG = /test/g;
console.log("Test with 'g' flag:");
console.log("First call:", patternWithG.test("test")); // true
console.log("LastIndex after first call:", patternWithG.lastIndex); // 4
console.log("Second call (same string):", patternWithG.test("test")); // false - fails!
console.log("LastIndex after second call:", patternWithG.lastIndex); // 0 (reset)
console.log("Third call:", patternWithG.test("test")); // true - succeeds again
// Test 2: Regex without g flag - no state
const patternWithoutG = /test/;
console.log("\nTest without 'g' flag:");
console.log("First call:", patternWithoutG.test("test")); // true
console.log("LastIndex:", patternWithoutG.lastIndex); // 0 (never changes)
console.log("Second call:", patternWithoutG.test("test")); // true
console.log("Third call:", patternWithoutG.test("test")); // true
EOF
Length of output: 268
🏁 Script executed (no clone):
#!/bin/bash
# Verify the suggested fix works correctly
node <<'EOF'
// Simulate the original code with g flag passed
function testOriginal() {
const pattern = /test/g; // Someone passes a regex with g flag
const texts = ["test", "test", "no match"];
for (let i = 0; i < texts.length; i++) {
const text = texts[i];
console.log(`Iteration ${i}: testing "${text}" with pattern.test() - Result:`, pattern.test(text));
}
}
// Simulate the suggested fix
function testFixed() {
const pattern = /test/g; // Someone passes a regex with g flag
const safePattern = new RegExp(pattern.source, pattern.flags.replace(/[gy]/g, ''));
const texts = ["test", "test", "no match"];
for (let i = 0; i < texts.length; i++) {
const text = texts[i];
console.log(`Iteration ${i}: testing "${text}" with safePattern.test() - Result:`, safePattern.test(text));
}
}
console.log("Original approach (buggy with g flag):");
testOriginal();
console.log("\nFixed approach (safe):");
testFixed();
EOF
Length of output: 495
Avoid stateful RegExp behavior in row matching.
The method accepts a RegExp parameter without protection against the g (global) and y (sticky) flags, which maintain state across iterations via lastIndex. Callers passing such patterns will experience skipped matches. While the current callsite uses a non-flagged pattern, defensive programming should ensure robustness.
Strip problematic flags to ensure consistent matching behavior across loop iterations:
Suggested fix
async clickFirstRowLinkMatching(pattern: RegExp): Promise<void> {
const links = this.dataViewCells.locator('a');
const count = await links.count();
+ const safePattern = new RegExp(pattern.source, pattern.flags.replace(/[gy]/g, ''));
for (let i = 0; i < count; i++) {
const text = await links.nth(i).textContent();
- if (text && pattern.test(text)) {
+ if (text && safePattern.test(text)) {
await this.robustClick(links.nth(i));
return;
}
}
throw new Error(`No row link matching ${pattern} found`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async clickFirstRowLinkMatching(pattern: RegExp): Promise<void> { | |
| const links = this.dataViewCells.locator('a'); | |
| const count = await links.count(); | |
| for (let i = 0; i < count; i++) { | |
| const text = await links.nth(i).textContent(); | |
| if (text && pattern.test(text)) { | |
| await this.robustClick(links.nth(i)); | |
| async clickFirstRowLinkMatching(pattern: RegExp): Promise<void> { | |
| const links = this.dataViewCells.locator('a'); | |
| const count = await links.count(); | |
| const safePattern = new RegExp(pattern.source, pattern.flags.replace(/[gy]/g, '')); | |
| for (let i = 0; i < count; i++) { | |
| const text = await links.nth(i).textContent(); | |
| if (text && safePattern.test(text)) { | |
| await this.robustClick(links.nth(i)); | |
| return; | |
| } | |
| } | |
| throw new Error(`No row link matching ${pattern} found`); | |
| } |
🤖 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/list-page.ts` around lines 54 - 60, The
clickFirstRowLinkMatching method can misbehave if callers pass a RegExp with
global or sticky flags because RegExp.lastIndex is stateful; defensively create
a fresh RegExp without the g or y flags from the incoming pattern before the
loop (use pattern.source and pattern.flags filtered to remove 'g' and 'y') and
then use that new RegExp for matching inside the loop, keeping the rest of the
logic (including robustClick) unchanged.
| const logsPage = new LogsPage(page); | ||
|
|
||
| await test.step('Navigate to kube-apiserver pods', async () => { | ||
| await page.goto('/k8s/ns/openshift-kube-apiserver/core~v1~Pod'); |
There was a problem hiding this comment.
Avoid absolute page.goto() paths in e2e routes
Line 65, 97, 135, 142, 149, and 152 use absolute paths (/k8s/...). This can break when Console is hosted behind a non-root proxy path. Please route via a base-path-safe helper/prefix.
As per coding guidelines: **/*.{ts,tsx,js,jsx}: Never use absolute paths in code; the app must be able to run behind a proxy under an arbitrary path.
Also applies to: 97-97, 135-135, 142-142, 149-149, 152-152
🤖 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/app/resource-log.spec.ts` at line 65, The test
uses absolute paths in page.goto (e.g.
page.goto('/k8s/ns/openshift-kube-apiserver/core~v1~Pod')) which breaks behind
non-root proxy; replace these calls with the base-path-safe routing helper used
elsewhere (for example a getConsoleRoute or buildConsoleUrl helper) so the path
is prefixed by the app's base path, and update each occurrence of
page.goto('/k8s/...') in resource-log.spec.ts to call that helper (e.g. await
page.goto(getConsoleRoute('k8s/ns/openshift-kube-apiserver/core~v1~Pod'))) so
tests work when Console is hosted under a non-root path.
| import { test, expect } from '../../../fixtures'; | ||
| import { CatalogPage } from '../../../pages/catalog-page'; | ||
|
|
||
| const TEMPLATE_NAME = 'httpd-example-test'; |
There was a problem hiding this comment.
Use a unique template name per run to avoid shared-cluster collisions.
Line 4 uses a fixed name in openshift; repeated/parallel runs can fail with AlreadyExists and make this spec flaky.
Proposed fix
-const TEMPLATE_NAME = 'httpd-example-test';
+const TEMPLATE_NAME = `httpd-example-test-${Date.now()}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const TEMPLATE_NAME = 'httpd-example-test'; | |
| const TEMPLATE_NAME = `httpd-example-test-${Date.now()}`; |
🤖 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/app/template.spec.ts` at line 4, The test uses a
fixed constant TEMPLATE_NAME ('httpd-example-test') causing collisions; update
the spec (template.spec.ts) to generate a unique template name at runtime (e.g.,
append a timestamp or UUID) and replace the constant TEMPLATE_NAME with that
dynamic value so each run uses a distinct name; ensure the generated name is
used wherever TEMPLATE_NAME is referenced in the test so creation calls no
longer clash with existing resources.
stefanonardo
force-pushed
the
CONSOLE-5235
branch
3 times, most recently
from
69aee6f to
42ef523
Compare
Migrate 6 Cypress test files (21 tests) from packages/integration-tests/tests/app/ to Playwright: - masthead (6 tests) - overview (2 tests) - node-terminal (1 test) - resource-log (3 tests) - template (1 test) - filtering-and-searching (8 tests) New page objects: list-page, details-page, logs-page, catalog-page, masthead-page, overview-page. Extended KubernetesClient with createPod, deletePod, waitForPodReady, createDeployment, waitForDeploymentReady. Deleted Cypress files and their exclusive dependencies (views/logs.ts, views/catalogs.ts, views/overview.ts, fixture YAMLs) after validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
42ef523 to
8810a34
Compare
|
@stefanonardo: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Analysis / Root cause:
CONSOLE-5235 — Migrate 6 Cypress test files (21 tests) from
packages/integration-tests/tests/app/to Playwright as part of the OCP 5.0 Playwright migration effort (CONSOLE-5196).Solution description:
Migrated all 6 basic app Cypress test files to idiomatic Playwright:
masthead.cy.tsmasthead.spec.tsoverview.cy.tsoverview.spec.tsnode-terminal.cy.tsnode-terminal.spec.tsresource-log.cy.tsresource-log.spec.tstemplate.cy.tstemplate.spec.tsfiltering-and-searching.cy.tsfiltering-and-searching.spec.tsNew page objects (reusable across future migrations):
ListPage— DataView table, filtering by name, row assertionsDetailsPage— resource details loading, tab navigationLogsPage— log viewer options, wrap toggle, container select, searchCatalogPage— catalog filtering, item/icon assertionsMastheadPage— logo, quick create, user dropdownOverviewPage— topology list view, sidebarKubernetesClient extensions:
createPod,deletePod,waitForPodReady,createDeployment,waitForDeploymentReadyKey translation decisions:
cy.exec('oc ...')withKubernetesClientAPI callscy.login()/cy.initAdmin()withstorageStateauthcy.wait(ms)with condition-based assertionscleanup.trackNamespace()Screenshots / screen recording:
Test setup:
Requires a running OpenShift cluster. Configure
frontend/e2e/.envwith cluster credentials, then run:Test cases:
--retries=0npx tsc --noEmit)Browser conformance:
Additional info:
details-page.ts,list-page.ts,modal.ts,yaml-editor.ts) are still used by other unmigrated tests and were intentionally kept🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation