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
📝 WalkthroughWalkthroughThis PR migrates E2E tests from Cypress to Playwright: it adds KubernetesClient pod/deployment lifecycle helpers, a BasePage retry helper, six Playwright page-objects (Catalog, Details, List, Logs, Masthead, Overview), and multiple new Playwright specs (filtering/searching, masthead, node terminal, overview, resource logs, template). It also adds WebSocket interceptions in cluster-settings tests, adjusts Playwright config and test selector attributes, removes legacy Cypress fixtures/helpers, and updates migration/docs and the debug-e2e skill metadata. Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (9 passed)
✨ 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.
stefanonardo
force-pushed
the
CONSOLE-5235
branch
4 times, most recently
from
42ef523 to
8810a34
Compare
openshift-ci
Bot
added
the
needs-rebase
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
8810a34 to
b1684ae
Compare
openshift-ci
Bot
removed
the
needs-rebase
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
b1684ae to
0d04f9a
Compare
stefanonardo
force-pushed
the
CONSOLE-5235
branch
4 times, most recently
from
fddfc04 to
b2d42e0
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/e2e/pages/masthead-page.ts (1)
19-24: ⚡ Quick winUse
robustClick()consistently for masthead interactions.Line 22, Line 33, Line 38, and Line 53 use direct
.click(). In these menu/dropdown flows, usingrobustClick()consistently will better absorb transient overlays/re-renders and reduce flake.As per coding guidelines
frontend/e2e/pages/**/*.ts: ExtendBasePagein Playwright page objects which providesrobustClick(),waitForLoadingComplete(), andgoTo().Also applies to: 26-35, 37-39, 48-54
🤖 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 19 - 24, The page object should extend BasePage and use its robustClick helper rather than direct .click() for masthead interactions: update the class declaration to extend BasePage, replace direct .click() calls (e.g. in async openQuickCreate() using quickCreateToggle and any other masthead locators referenced) with this.robustClick(locator), and ensure you rely on BasePage methods like waitForLoadingComplete() and goTo() where appropriate to follow the project e2e guidelines.
🤖 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/debug-e2e/SKILL.md:
- Around line 2-10: Update the markdown heading to match the renamed skill by
replacing the existing "# Debug Test" heading with a heading that matches the
skill name "debug-e2e" (for example "# debug-e2e" or a more descriptive "# Debug
E2E (debug-e2e)") so the top-level heading aligns with the name: debug-e2e
declared in the file; locate the heading in the SKILL.md content and edit it
accordingly.
In `@frontend/e2e/pages/list-page.ts`:
- Around line 8-15: Replace the brittle class/OUIA locators with test-id-based
Playwright selectors: update dataViewCells, dataViewFilters, nameFilterInput,
and singleFilterGroup to use page.getByTestId('...') querying [data-test="..."]
(choose meaningful test ids like "data-view-cell", "data-view-filters",
"name-filter-input", "single-filter-group"); if any target element lacks a
data-test attribute, add data-test in the UI component (keep existing
legacy/class/OUIA attributes), and update any other similar locators in the file
(e.g., the ones noted around the rest of the file) to follow the same
page.getByTestId convention.
---
Nitpick comments:
In `@frontend/e2e/pages/masthead-page.ts`:
- Around line 19-24: The page object should extend BasePage and use its
robustClick helper rather than direct .click() for masthead interactions: update
the class declaration to extend BasePage, replace direct .click() calls (e.g. in
async openQuickCreate() using quickCreateToggle and any other masthead locators
referenced) with this.robustClick(locator), and ensure you rely on BasePage
methods like waitForLoadingComplete() and goTo() where appropriate to follow the
project e2e guidelines.
🪄 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: e705fb20-6f31-4e42-939b-49c9a13120d5
📒 Files selected for processing (15)
.claude/skills/debug-e2e/SKILL.mdfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/base-page.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.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- frontend/e2e/tests/console/app/overview.spec.ts
- frontend/e2e/pages/overview-page.ts
- frontend/e2e/tests/console/app/node-terminal.spec.ts
- frontend/e2e/clients/kubernetes-client.ts
- frontend/e2e/pages/details-page.ts
- frontend/e2e/tests/console/app/masthead.spec.ts
- frontend/e2e/pages/logs-page.ts
- frontend/e2e/tests/console/app/filtering-and-searching.spec.ts
- frontend/e2e/tests/console/app/resource-log.spec.ts
- frontend/e2e/pages/catalog-page.ts
- frontend/e2e/tests/console/app/template.spec.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx,js,jsx}: Never import from package index files (e.g.,@console/shared) in new code, as they can create circular dependencies and slow builds. Import from specific file paths instead.
Do not use backticks int()calls for i18n strings, as the i18n parser cannot extract keys from template literals. Use single or double quotes instead.
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import from deprecated packages or use code with the
@deprecatedTSdoc tag in new code.
**/*.{ts,tsx}: Use React functional components with hooks instead of class components
State Management: Use React hooks and Context API (migrating away from legacy Redux/Immutable.js)
Hooks: Use existing hooks fromconsole-sharedwhen possible (useK8sWatchResource,useUserSettings, etc.)
API calls: Use k8s resource hooks for data fetching,consoleFetchJSONfor HTTP requests
Extensions: Use console extension points for plugin integration
Types: Check existing types inconsole-sharedbefore creating new ones
Dynamic Plugins: Use console extension points for plugin integration
Styling: Use SCSS modules co-located with components, PatternFly design system components, avoid any SCSS/CSS if possible
Accessibility: Follow WCAG 2.1 AA standards, use semantic HTML, ARIA labels where needed, ensure keyboard navigation, test with screen readers
i18n: UseuseTranslation('namespace')hook withkeyformat for translation keys
Error Handling: Use ErrorBoundary components and graceful degradation patterns
Optimize re-renders: UseuseCallbackfor memoized callbacks to avoid function recreation every render
Optimize re-renders: UseuseMemofor expensive computations to avoid recalculating on every render
Lazy loading: UseReact.lazy()to lazy load heavy components
TypeScript type safety: Avoid usinganytype; suggest proper type definitions and verify null/undefined are handled properly
Type component props properly: Reuse existing component prop types instead of duplicating type definitions
Use proper hooks: Use specialized hooks likeusePluginInfofor plugin data instead of generic data fetching patterns
Avoid deprecated components: Check for JSDoc@deprecatedtags, import paths containing/deprecated, andDEPRECATED_file name prefix before using components
Importing from barrel files and circular dependencies: Import directly from specific files instead...
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
frontend/**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Never use absolute URLs or paths in the console code. The console runs behind a proxy under an arbitrary path.
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
When writing code for static plugins, ensure that all
$codeRefreference the corresponding extension type from the@console/dynamic-plugin-sdkpackage.
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (TESTING.md)
**/*.{tsx,ts}: Always usepage.getByTestId('x')for Playwright selectors which queries[data-test="x"]. If a React element only has a legacy test attribute, adddata-testto the element. Never remove legacy attributes
Preferdata-testattributes in Cypress selectors (e.g.,cy.get('[data-test="create-deployment"]')) over brittle CSS/ARIA selectorsFile Naming: PascalCase for components, kebab-case for utilities,
*.spec.ts(x)for tests
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
frontend/e2e/pages/**/*.ts
📄 CodeRabbit inference engine (TESTING.md)
Extend
BasePagein Playwright page objects which providesrobustClick(),waitForLoadingComplete(), andgoTo(). Locators areprivate readonlyproperties; actions areasyncmethods
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
**/*.{go,ts,tsx,js,jsx}
📄 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/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
**/*.{ts,tsx,js,jsx}: New code MUST be written in TypeScript, not JavaScript
Prefer functional programming patterns and immutable data structures
Run the linter and follow all rules defined in .eslintrc
Never use absolute paths in code - the app should be able to run behind a proxy under an arbitrary path
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
**/*.ts
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Plugin SDK Changes: Any updates to
console-dynamic-plugin-sdkshould aim to maintain backward compatibility as it's a public API - use theplugin-api-reviewskill to vet changes for public API impact and ensure proper documentation updates
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
frontend/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (README.md)
frontend/**/*.{js,ts,tsx}: Support only the latest versions of Edge, Chrome, Safari, and Firefox browsers; IE 11 and earlier are not supported
CSP violations should be automatically reported to telemetry by parsing dynamic plugin names from securitypolicyviolation events, with throttling to prevent duplicate reports within a day
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (INTERNATIONALIZATION.md)
For dynamic translation keys that cannot be parsed by i18next-parser (t(key), t('key' + id), t(
key${id})), specify possible static values in comments for the parser to extract
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/list-page.ts
🪛 ast-grep (0.42.2)
frontend/e2e/pages/list-page.ts
[warning] 64-64: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern.source, safeFlags)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 LanguageTool
.claude/skills/debug-e2e/SKILL.md
[style] ~105-~105: 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 ...
(ADVERB_WORD_ORDER)
🔇 Additional comments (3)
frontend/e2e/pages/base-page.ts (1)
36-39: LGTM!frontend/e2e/pages/masthead-page.ts (1)
5-18: LGTM!Also applies to: 41-47, 56-59
frontend/e2e/pages/list-page.ts (1)
63-76: LGTM!
|
/retest |
stefanonardo
force-pushed
the
CONSOLE-5235
branch
2 times, most recently
from
a4685f9 to
926c556
Compare
openshift-ci
Bot
added
the
needs-rebase
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
926c556 to
9645a8a
Compare
openshift-ci
Bot
removed
the
needs-rebase
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
9645a8a to
305bb8d
Compare
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
305bb8d to
15cbcab
Compare
| expect: { | ||
| timeout: 40_000, |
There was a problem hiding this comment.
I think we should keep the default value to speed up the (failing) tests and increase it only when needed
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
15cbcab to
19e532c
Compare
|
@CodeRabbit help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
frontend/e2e/pages/logs-page.ts (1)
7-15: ⚡ Quick winKeep page-object locators private and expose intent methods/assertion helpers instead.
lineCount,searchMatches, andlogTextare public locators. Please keep locatorsprivate readonlyand expose dedicated methods for assertions/actions to preserve page-object boundaries.As per coding guidelines: “frontend/e2e/pages/**/*.ts: … Locators are
private readonlyproperties; actions areasyncmethods.”🤖 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 7 - 15, Change the public locators lineCount, searchMatches, and logText to private readonly properties and replace external access with intent-focused methods on the page object: add an async getLineCount() or assertLineCount(expected) to read/validate lineCount, add async getSearchMatchesCount() or assertHasSearchMatches(count) to interact/assert searchMatches, and add async getLogText() or assertLogContains(text) to read/assert logText; update any tests to call these new methods instead of referencing the locators directly so the page-object keeps locators private (refer to the existing properties lineCount, searchMatches, logText and the page class where they are declared to locate and modify them).frontend/e2e/tests/console/app/masthead.spec.ts (1)
52-55: ⚡ Quick winUse a narrower assertion target than
bodyfor token-page verification.Line 54 can pass on unrelated page content. Prefer asserting a specific element (ideally
getByTestId(...)) on the token display page to reduce false positives.🤖 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 52 - 55, The token display page verification is using a broad `body` text assertion that can match unrelated content. Update the `Verify token display page` step in `masthead.spec.ts` to assert against a more specific element on the token page, preferably via `page.getByTestId(...)` (or another unique locator) instead of `page.locator('body')`, so the check is tied to the actual token display UI.frontend/e2e/pages/details-page.ts (1)
9-13: ⚡ Quick winKeep page-object locators private and expose intent via methods/getters only.
nodeTerminalErrorandxtermViewportare currently public locator fields. For consistency with the page-object contract, make locatorsprivate readonlyand expose access through explicit getters/actions.As per coding guidelines: “
frontend/e2e/pages/**/*.ts: Locators areprivate readonlyproperties; actions areasyncmethods”.🤖 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/details-page.ts` around lines 9 - 13, Change the public locator fields nodeTerminalError and xtermViewport to private readonly properties and replace any direct usage with public getters or async action methods that return or act on these locators; update their declarations in the class (e.g., change "nodeTerminalError: Locator" and "xtermViewport: Locator" to "private readonly nodeTerminalError" and "private readonly xtermViewport") and add explicit getters/actions (e.g., getNodeTerminalError() or async hasNodeTerminalError(), getXtermViewport(), etc.) so callers use the intent-based API instead of accessing locators directly (ensure existing usages of resourceTitle remain consistent with the getter pattern).
🤖 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/catalog-page.ts`:
- Around line 6-23: Replace brittle selectors by switching the filterInput and
catalogItemIcon locators to use getByTestId backed by data-test attributes:
update the private filterInput (used in navigateToCatalog and filterByKeyword)
to this.page.getByTestId('your-filter-testid') and change catalogItemIcon (used
by catalogItemIcon(testId)) to return
this.catalogItem(testId).getByTestId('catalog-icon-testid') (or a per-item
data-test attribute). Ensure the page elements have corresponding data-test
attributes in the app, and keep the existing waitFor/timeout logic in
navigateToCatalog and fill logic in filterByKeyword untouched.
In `@frontend/e2e/pages/logs-page.ts`:
- Around line 13-15: Replace fragile placeholder/class selectors in the LogsPage
by switching the SearchLogs selectors to stable data-test hooks: change usages
of this.page.getByPlaceholder('Search logs') to
this.page.getByTestId('search-logs') for the searchInput, replace
this.page.locator('.pf-m-match') with this.page.getByTestId('log-match') for
searchMatches, and replace
this.page.locator('span[class$="c-log-viewer__text"]') with
this.page.getByTestId('log-text') for logText; if the corresponding React
elements lack data-test attributes add data-test="search-logs",
data-test="log-match", and data-test="log-text" to those elements so the
Playwright selectors are stable.
In `@frontend/e2e/pages/overview-page.ts`:
- Around line 52-67: labelCell currently uses a substring hasText match which
can return multiple rows; update labelCell(name) to perform an exact-text match
(e.g., use getByText(name, { exact: true }) or a hasText regex anchored to
^name$) so it only selects the intended cell, and update clickListItem(name) to
call .first() or assert there is exactly one match before .click() to avoid
flakiness; target the labelCell and clickListItem functions for this change.
In `@frontend/e2e/tests/console/app/filtering-and-searching.spec.ts`:
- Around line 20-62: The test suite currently creates shared resources in
test.beforeAll (ns, workloadName, createClient, waitForDeploymentReady) and
deletes them in test.afterAll; change this to per-test lifecycle by moving
namespace/deployment creation into each test or test.beforeEach and register
cleanup with the cleanup fixture so each test gets its own ns/workloadName and
the cleanup fixture deletes them automatically; replace usages of
test.beforeAll/test.afterAll that create/delete the namespace and deployment
with per-test creation (using createClient, createNamespace, createDeployment,
waitForDeploymentReady) and call cleanup.add or the appropriate cleanup fixture
method to delete the namespace/workload after each test.
---
Nitpick comments:
In `@frontend/e2e/pages/details-page.ts`:
- Around line 9-13: Change the public locator fields nodeTerminalError and
xtermViewport to private readonly properties and replace any direct usage with
public getters or async action methods that return or act on these locators;
update their declarations in the class (e.g., change "nodeTerminalError:
Locator" and "xtermViewport: Locator" to "private readonly nodeTerminalError"
and "private readonly xtermViewport") and add explicit getters/actions (e.g.,
getNodeTerminalError() or async hasNodeTerminalError(), getXtermViewport(),
etc.) so callers use the intent-based API instead of accessing locators directly
(ensure existing usages of resourceTitle remain consistent with the getter
pattern).
In `@frontend/e2e/pages/logs-page.ts`:
- Around line 7-15: Change the public locators lineCount, searchMatches, and
logText to private readonly properties and replace external access with
intent-focused methods on the page object: add an async getLineCount() or
assertLineCount(expected) to read/validate lineCount, add async
getSearchMatchesCount() or assertHasSearchMatches(count) to interact/assert
searchMatches, and add async getLogText() or assertLogContains(text) to
read/assert logText; update any tests to call these new methods instead of
referencing the locators directly so the page-object keeps locators private
(refer to the existing properties lineCount, searchMatches, logText and the page
class where they are declared to locate and modify them).
In `@frontend/e2e/tests/console/app/masthead.spec.ts`:
- Around line 52-55: The token display page verification is using a broad `body`
text assertion that can match unrelated content. Update the `Verify token
display page` step in `masthead.spec.ts` to assert against a more specific
element on the token page, preferably via `page.getByTestId(...)` (or another
unique locator) instead of `page.locator('body')`, so the check is tied to the
actual token display UI.
🪄 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: 4c164179-698f-4b3c-bd45-289be55dd674
📒 Files selected for processing (37)
.claude/migration-context.md.claude/skills/debug-e2e/SKILL.mdfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/base-page.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/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.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.tsfrontend/playwright.config.tsfrontend/public/components/utils/container-select.tsxfrontend/public/components/utils/resource-log.tsx
💤 Files with no reviewable changes (13)
- frontend/packages/integration-tests/views/overview.ts
- frontend/packages/integration-tests/tests/app/masthead.cy.ts
- frontend/packages/integration-tests/tests/app/filtering-and-searching.cy.ts
- frontend/packages/integration-tests/tests/app/node-terminal.cy.ts
- frontend/playwright.config.ts
- frontend/packages/integration-tests/tests/app/overview.cy.ts
- frontend/packages/integration-tests/views/catalogs.ts
- frontend/packages/integration-tests/tests/app/resource-log.cy.ts
- frontend/packages/integration-tests/views/logs.ts
- frontend/packages/integration-tests/fixtures/pod-with-wrap-annotation.yaml
- frontend/packages/integration-tests/tests/app/template.cy.ts
- frontend/packages/integration-tests/fixtures/httpd-example-template.yaml
- frontend/packages/integration-tests/fixtures/pod-with-space.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx,js,jsx}: Never import from package index files (e.g.,@console/shared) in new code, as they can create circular dependencies and slow builds. Import from specific file paths instead.
Do not use backticks int()calls for i18n strings, as the i18n parser cannot extract keys from template literals. Use single or double quotes instead.
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import from deprecated packages or use code with the
@deprecatedTSdoc tag in new code.
**/*.{ts,tsx}: Use React functional components with hooks instead of class components
State Management: Use React hooks and Context API (migrating away from legacy Redux/Immutable.js)
Hooks: Use existing hooks fromconsole-sharedwhen possible (useK8sWatchResource,useUserSettings, etc.)
API calls: Use k8s resource hooks for data fetching,consoleFetchJSONfor HTTP requests
Extensions: Use console extension points for plugin integration
Types: Check existing types inconsole-sharedbefore creating new ones
Dynamic Plugins: Use console extension points for plugin integration
Styling: Use SCSS modules co-located with components, PatternFly design system components, avoid any SCSS/CSS if possible
Accessibility: Follow WCAG 2.1 AA standards, use semantic HTML, ARIA labels where needed, ensure keyboard navigation, test with screen readers
i18n: UseuseTranslation('namespace')hook withkeyformat for translation keys
Error Handling: Use ErrorBoundary components and graceful degradation patterns
Optimize re-renders: UseuseCallbackfor memoized callbacks to avoid function recreation every render
Optimize re-renders: UseuseMemofor expensive computations to avoid recalculating on every render
Lazy loading: UseReact.lazy()to lazy load heavy components
TypeScript type safety: Avoid usinganytype; suggest proper type definitions and verify null/undefined are handled properly
Type component props properly: Reuse existing component prop types instead of duplicating type definitions
Use proper hooks: Use specialized hooks likeusePluginInfofor plugin data instead of generic data fetching patterns
Avoid deprecated components: Check for JSDoc@deprecatedtags, import paths containing/deprecated, andDEPRECATED_file name prefix before using components
Importing from barrel files and circular dependencies: Import directly from specific files instead...
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
frontend/**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Never use absolute URLs or paths in the console code. The console runs behind a proxy under an arbitrary path.
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
When writing code for static plugins, ensure that all
$codeRefreference the corresponding extension type from the@console/dynamic-plugin-sdkpackage.
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
frontend/e2e/tests/**/*.spec.ts
📄 CodeRabbit inference engine (TESTING.md)
frontend/e2e/tests/**/*.spec.ts: E2E tests must validate full user workflows against a real OpenShift cluster using Playwright, located infrontend/e2e/tests/<package>/
Each Playwright E2E test block must create its own resources, assert independently, and clean up via thecleanupfixture
UseKubernetesClientfor cluster interactions in Playwright tests - never use shell commands in tests
Importtestandexpectfrome2e/fixtures, not from@playwright/test. Custom fixtures providecleanup,testConfig, andk8sClient
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (TESTING.md)
**/*.{tsx,ts}: Always usepage.getByTestId('x')for Playwright selectors which queries[data-test="x"]. If a React element only has a legacy test attribute, adddata-testto the element. Never remove legacy attributes
Preferdata-testattributes in Cypress selectors (e.g.,cy.get('[data-test="create-deployment"]')) over brittle CSS/ARIA selectorsFile Naming: PascalCase for components, kebab-case for utilities,
*.spec.ts(x)for tests
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
**/*.{go,ts,tsx,js,jsx}
📄 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/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
**/*.{ts,tsx,js,jsx}: New code MUST be written in TypeScript, not JavaScript
Prefer functional programming patterns and immutable data structures
Run the linter and follow all rules defined in .eslintrc
Never use absolute paths in code - the app should be able to run behind a proxy under an arbitrary path
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Tests should follow a similar 'test tables' convention as used in Go where applicable
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.ts
**/*.ts
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Plugin SDK Changes: Any updates to
console-dynamic-plugin-sdkshould aim to maintain backward compatibility as it's a public API - use theplugin-api-reviewskill to vet changes for public API impact and ensure proper documentation updates
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
frontend/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (README.md)
frontend/**/*.{js,ts,tsx}: Support only the latest versions of Edge, Chrome, Safari, and Firefox browsers; IE 11 and earlier are not supported
CSP violations should be automatically reported to telemetry by parsing dynamic plugin names from securitypolicyviolation events, with throttling to prevent duplicate reports within a day
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (INTERNATIONALIZATION.md)
For dynamic translation keys that cannot be parsed by i18next-parser (t(key), t('key' + id), t(
key${id})), specify possible static values in comments for the parser to extract
Files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/public/components/utils/container-select.tsxfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/public/components/utils/resource-log.tsxfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
frontend/e2e/pages/**/*.ts
📄 CodeRabbit inference engine (TESTING.md)
Extend
BasePagein Playwright page objects which providesrobustClick(),waitForLoadingComplete(), andgoTo(). Locators areprivate readonlyproperties; actions areasyncmethods
Files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
🧠 Learnings (4)
📚 Learning: 2026-05-18T11:35:14.316Z
Learnt from: stefanonardo
Repo: openshift/console PR: 16431
File: frontend/e2e/tests/console/app/resource-log.spec.ts:65-65
Timestamp: 2026-05-18T11:35:14.316Z
Learning: In OpenShift Console Playwright e2e tests, when navigating to in-console routes use `page.goto('/k8s/...')` with a leading-slash absolute path (e.g., `/k8s/...`). `WEB_CONSOLE_URL` (default `http://localhost:9000`) must be treated as an origin only (no path component), so Playwright’s `baseURL` correctly resolves absolute paths. This convention should be followed consistently in console e2e specs when calling `page.goto()`, rather than passing full URLs or including path segments in `WEB_CONSOLE_URL`.
Applied to files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.ts
📚 Learning: 2026-05-19T17:15:42.558Z
Learnt from: fsgreco
Repo: openshift/console PR: 16461
File: frontend/e2e/pages/web-terminal-config-page.ts:18-18
Timestamp: 2026-05-19T17:15:42.558Z
Learning: In the openshift/console repo, the guideline “Never use absolute URLs or paths in the console code” should be treated as an application-source-only rule. Do not flag absolute-root paths/URLs (e.g., strings starting with `/` like `/k8s/cluster/...`) in Playwright E2E code under `frontend/e2e/`, including page objects (e.g., `page.goto()` / `goTo()` calls). These absolute paths are considered correct and intentional because Playwright resolves them against the `baseURL` configured in `frontend/e2e/playwright.config.ts`. Only enforce the no-absolute-paths rule in application code under `frontend/public` and `frontend/packages`.
Applied to files:
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.tsfrontend/e2e/tests/console/cluster-settings/updates-graph.spec.tsfrontend/e2e/tests/console/app/template.spec.tsfrontend/e2e/tests/console/cluster-settings/channel-modal.spec.tsfrontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.tsfrontend/e2e/tests/console/app/overview.spec.tsfrontend/e2e/tests/console/app/node-terminal.spec.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.tsfrontend/e2e/tests/console/app/masthead.spec.tsfrontend/e2e/tests/console/app/resource-log.spec.tsfrontend/e2e/tests/console/cluster-settings/update-modal.spec.tsfrontend/e2e/tests/console/app/filtering-and-searching.spec.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
📚 Learning: 2026-05-20T13:09:57.325Z
Learnt from: fsgreco
Repo: openshift/console PR: 16461
File: frontend/e2e/pages/web-terminal-page.ts:59-59
Timestamp: 2026-05-20T13:09:57.325Z
Learning: In page object files under `frontend/e2e/pages/`, do not treat `.catch(() => {})` (or other empty/ignored catches) chained to `waitFor(...)` as a bug when the wait is intended to be best-effort. These catches indicate the element may or may not appear, so the page object should not fail the test; real expectations should be asserted in the spec files (e.g., `expect(...).toBeVisible()`). This is consistent with the `waitForLoadingComplete` behavior in `BasePage`—reviewers should allow the empty catches when they match this pattern.
Applied to files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
📚 Learning: 2026-05-20T13:21:50.712Z
Learnt from: fsgreco
Repo: openshift/console PR: 16461
File: frontend/e2e/pages/web-terminal-page.ts:11-26
Timestamp: 2026-05-20T13:21:50.712Z
Learning: In Playwright page objects under `frontend/e2e/pages/`, treat locators that intentionally use dual CSS selectors like `.locator('[data-test="X"], [data-test-id="X"]')` as a backwards-compatibility shim. The legacy `data-test-id` is required because newly added `data-test` attributes may not exist on yet-to-be-upgraded deployed clusters. Do not flag these dual-selector locators as a code smell and do not suggest replacing them with a single `getByTestId('X')` until the new `data-test` attributes have propagated to all target clusters.
Applied to files:
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/catalog-page.tsfrontend/e2e/pages/logs-page.tsfrontend/e2e/pages/details-page.tsfrontend/e2e/pages/masthead-page.tsfrontend/e2e/pages/overview-page.tsfrontend/e2e/pages/list-page.ts
🪛 ast-grep (0.42.2)
frontend/e2e/pages/list-page.ts
[warning] 64-64: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern.source, safeFlags)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 LanguageTool
.claude/skills/debug-e2e/SKILL.md
[style] ~105-~105: 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 ...
(ADVERB_WORD_ORDER)
🔇 Additional comments (13)
frontend/e2e/tests/console/cluster-settings/update-in-progress.spec.ts (1)
6-6: LGTM!Also applies to: 12-15
frontend/e2e/tests/console/cluster-settings/updates-graph.spec.ts (1)
5-5: LGTM!Also applies to: 98-101
frontend/e2e/tests/console/app/template.spec.ts (1)
1-61: LGTM!frontend/public/components/utils/container-select.tsx (1)
26-26: LGTM!frontend/e2e/tests/console/cluster-settings/channel-modal.spec.ts (1)
9-9: LGTM!Also applies to: 15-18
frontend/e2e/tests/console/cluster-settings/worker-mcp-paused.spec.ts (1)
6-6: LGTM!Also applies to: 15-18
frontend/e2e/tests/console/app/overview.spec.ts (1)
1-155: LGTM!frontend/e2e/tests/console/app/node-terminal.spec.ts (1)
1-32: LGTM!frontend/e2e/pages/base-page.ts (1)
36-39: LGTM!.claude/migration-context.md (1)
265-274: LGTM!.claude/skills/debug-e2e/SKILL.md (1)
2-10: LGTM!Also applies to: 19-31, 93-107
frontend/e2e/tests/console/cluster-settings/upgradeable-false.spec.ts (1)
6-6: LGTM!Also applies to: 12-15
frontend/e2e/tests/console/app/resource-log.spec.ts (1)
85-157: LGTM!
| private readonly filterInput: Locator = this.page.getByPlaceholder('Filter by keyword'); | ||
|
|
||
| async navigateToCatalog(): Promise<void> { | ||
| await this.goTo('/catalog/all-namespaces'); | ||
| await this.filterInput.waitFor({ state: 'visible', timeout: 60_000 }); | ||
| } | ||
|
|
||
| async filterByKeyword(keyword: string): Promise<void> { | ||
| await this.filterInput.fill(keyword); | ||
| } | ||
|
|
||
| catalogItem(testId: string): Locator { | ||
| return this.page.getByTestId(testId); | ||
| } | ||
|
|
||
| catalogItemIcon(testId: string): Locator { | ||
| return this.catalogItem(testId).locator('img.catalog-tile-pf-icon'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Prefer getByTestId/data-test selectors for new catalog locators.
Line 6 (getByPlaceholder) and Line 22 (CSS class selector) make this page object more brittle than necessary. Please switch these to getByTestId(...) backed by stable data-test attributes.
As per coding guidelines: “Always use page.getByTestId('x') for Playwright selectors… If a React element only has a legacy test attribute, add data-test to the element.”
🤖 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 - 23, Replace brittle
selectors by switching the filterInput and catalogItemIcon locators to use
getByTestId backed by data-test attributes: update the private filterInput (used
in navigateToCatalog and filterByKeyword) to
this.page.getByTestId('your-filter-testid') and change catalogItemIcon (used by
catalogItemIcon(testId)) to return
this.catalogItem(testId).getByTestId('catalog-icon-testid') (or a per-item
data-test attribute). Ensure the page elements have corresponding data-test
attributes in the app, and keep the existing waitFor/timeout logic in
navigateToCatalog and fill logic in filterByKeyword untouched.
There was a problem hiding this comment.
still better @stefanonardo here you could use getByRole('img') if there's only one image in the tile. Minor improvement though, non blocker.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@fsgreco the icon <img> has alt="" so it won't appear in the accessibility tree. Setting a non-empty value requires too many changes which I think are out of scope for this PR
| private readonly searchInput: Locator = this.page.getByPlaceholder('Search logs'); | ||
| readonly searchMatches: Locator = this.page.locator('.pf-m-match'); | ||
| readonly logText: Locator = this.page.locator('span[class$="c-log-viewer__text"]'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace placeholder/class selectors with stable data-test selectors.
Line 13 and Lines 14-15 rely on placeholder/CSS-class selectors, which are fragile for long-term e2e stability. Prefer getByTestId(...) with explicit data-test hooks.
As per coding guidelines: “Always use page.getByTestId('x') for Playwright selectors… If a React element only has a legacy test attribute, add data-test to the element.”
🤖 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 13 - 15, Replace fragile
placeholder/class selectors in the LogsPage by switching the SearchLogs
selectors to stable data-test hooks: change usages of
this.page.getByPlaceholder('Search logs') to
this.page.getByTestId('search-logs') for the searchInput, replace
this.page.locator('.pf-m-match') with this.page.getByTestId('log-match') for
searchMatches, and replace
this.page.locator('span[class$="c-log-viewer__text"]') with
this.page.getByTestId('log-text') for logText; if the corresponding React
elements lack data-test attributes add data-test="search-logs",
data-test="log-match", and data-test="log-text" to those elements so the
Playwright selectors are stable.
6999b55 to
f7a5e3c
Compare
logonoff
left a comment
logonoff
left a comment
There was a problem hiding this comment.
/label px-approved
/label docs-approved
openshift-ci
Bot
added
px-approved
stefanonardo
force-pushed
the
CONSOLE-5235
branch
from
f7a5e3c to
0b7e476
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
0b7e476 to
598324e
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fsgreco, logonoff, 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 |
|
@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. |
4 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 navigation, kebab actions (merged with upstream additions)LogsPage— log viewer options, wrap toggle, container select, searchCatalogPage— catalog filtering, item/icon assertionsMastheadPage— logo, quick create, user dropdownOverviewPage— cluster overview + topology list view, sidebar (merged with upstream additions)KubernetesClient extensions:
createPod,deletePod,waitForPodReady(with container readiness check),createDeployment,waitForDeploymentReadyKey translation decisions:
cy.exec('oc ...')withKubernetesClientAPI callscy.login()/cy.initAdmin()withstorageStateauthcy.wait(ms)with condition-based assertionscleanup.trackNamespace()Partial<V1Pod>/Partial<V1Deployment>types to avoid unsafeas anycastsDate.now()) to prevent collisionsFixes to existing cluster-settings tests:
Tests that use
page.route()to mock Kubernetes API responses (e.g. ClusterVersion, MachineConfigPool) were failing because the console'swatchK8sObjectmakes two requests per watched resource: an HTTP GET (intercepted bypage.route()) and a WebSocket watch (not intercepted). The WebSocket delivered real cluster data that overwrote the mocked GET response in the Redux store.Fix: Added
page.routeWebSocket(pattern, () => {})calls alongside the existingpage.route()mocks. When the handler doesn't callconnectToServer(), Playwright creates a mock WebSocket that appears open to the page but never delivers server messages. The URL patterns are specific (e.g./apis\/config\.openshift\.io\/v1\/clusterversions/), so only matching WebSocket connections are intercepted — all other WebSockets pass through normally.In
update-modal.spec.ts, the previous 45-linestubMachineConfigPoolWebSocket()function usedpage.addInitScript()to monkey-patch the globalWebSocketconstructor. This was replaced with two idiomatic one-liners usingpage.routeWebSocket()(available since Playwright 1.48; project uses 1.59+), which provides the same selective interception behavior without modifying global browser state.Affected files:
channel-modal.spec.ts,update-in-progress.spec.ts,upgradeable-false.spec.ts,updates-graph.spec.ts,update-modal.spec.ts,worker-mcp-paused.spec.ts.Additional page object fixes:
overview-page.ts: FixedlabelCell()— the.odc-topology-list-view__label-cellelement contains both the kind badge prefix (e.g. "DaemonSetD") and the resource name, so the regex^name$never matched. Changed to substring match.catalog-page.ts: FixedcatalogItemIcon()— PatternFly's CatalogTile renders<img alt="">, which per ARIA spec hasrole="presentation", sogetByRole('img')correctly skips it. Changed to CSS selectorimg.catalog-tile-pf-icon.Screenshots / screen recording:
Test setup:
Requires a running OpenShift cluster. Configure
frontend/e2e/.envwith cluster credentials, then run:Test cases:
--retries=0npx tsc --noEmit)eslint-plugin-playwrightrulesBrowser 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