CONSOLE-5281: Migrate Cypress E2E tests to Playwright (batch 2)#16454
CONSOLE-5281: Migrate Cypress E2E tests to Playwright (batch 2)#16454rhamilto wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@rhamilto: This pull request references CONSOLE-5281 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
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
component/core
rhamilto
force-pushed
the
CONSOLE-5281
branch
3 times, most recently
from
074fbfd to
0b4f078
Compare
rhamilto
changed the title
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/unhold |
|
tech debt |
openshift-ci
Bot
added
px-approved
openshift-ci-robot
added
the
verified
|
@rhamilto: This PR has been marked as verified by 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. |
|
/retest |
openshift-ci-robot
removed
the
verified
|
@rhamilto: This pull request references CONSOLE-5281 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. |
|
|
||
| await test.step('Filter events by Warning type', async () => { | ||
| await page.getByTestId('console-select-menu-toggle').click(); | ||
| await page.locator('[data-test-dropdown-menu="warning"]').click(); |
There was a problem hiding this comment.
In order to maintain compatibility with other existing Cypress tests, need to use this specific selelctor.
|
|
||
| await test.step('Create a new namespace from the dropdown', async () => { | ||
| await page.getByTestId('dropdown-selectbox').click(); | ||
| await page.locator('[data-test-dropdown-menu^="Create_"]').click(); |
There was a problem hiding this comment.
In order to maintain compatibility with other existing Cypress tests, need to use this specific selelctor.
📝 WalkthroughWalkthroughThis PR achieves a comprehensive migration of end-to-end tests from Cypress to Playwright across multiple console features. It establishes new Playwright infrastructure (Kubernetes client extensions, page objects, test utilities), implements 20+ new Playwright test specs covering Alertmanager receivers, CRD extensions, Events, Favorites, Pseudolocalization, and OLM flows, removes the corresponding Cypress implementations, and polishes components with data-test selectors and a null-safety fix in the CRD printer columns hook. Suggested reviewers
🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (2)
frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts (1)
13-23: ⚡ Quick winStandardize these Playwright locators on
getByTestId.Placeholder/role/regex selectors here are fragile compared to the repo’s required test-id strategy. Please migrate these selectors and add missing
data-testhooks where needed.As per coding guidelines: "
**/*.{tsx,ts}: Always usepage.getByTestId('x')for Playwright selectors ... If a React element only has a legacy test attribute, adddata-testto the element."Also applies to: 34-45, 147-148
🤖 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/crd-extensions/crd-test-utils.ts` around lines 13 - 23, Replace fragile placeholder/role/regex selectors with test-id based selectors: use page.getByTestId(...) instead of page.locator('input[placeholder="Filter by name"]') for searchInput, replace the row match (crdRow) that uses getByRole with a test-id on the row element and fetch it via getByTestId, and swap the kebabButton lookup to getByTestId('kebab-button'); update the menu action lookup to use a test-id on the "View instances" menu item (or use getByTestId for the menu) and click via that selector. Add the corresponding data-test/data-test-id attributes to the React components rendering the search input, CRD row, kebab button, and "View instances" menu item so tests can use page.getByTestId consistently, and apply the same replacements to the other similar selectors elsewhere in this file.frontend/e2e/pages/alertmanager-page.ts (1)
39-39: ⚡ Quick winUse
getByTestIdselectors instead of role/text selectors in Playwright helpers.These locators are currently role/name-based and are more brittle than the repo standard. Please switch to
page.getByTestId(...)and add missingdata-testhooks where needed.As per coding guidelines: "
**/*.{tsx,ts}: Always usepage.getByTestId('x')for Playwright selectors ... If a React element only has a legacy test attribute, adddata-testto the element."Also applies to: 116-131
🤖 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/alertmanager-page.ts` at line 39, Replace brittle role/name-based Playwright locators that call this.page.getByRole('button', { name: 'Copy code to clipboard' }) (and similar occurrences around the other locators) with page.getByTestId('...') calls; add matching data-test attributes (e.g., data-test="copy-code-button") to the corresponding React elements where those buttons/texts are rendered so the new selectors can target them, update the helper(s) in alertmanager-page.ts to use page.getByTestId for all detections referenced (including the occurrences around the other block noted), and ensure test IDs follow the repo convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/e2e/clients/kubernetes-client.ts`:
- Around line 492-496: createPod uses non-null assertions on
pod.metadata!.namespace! and can throw unclear runtime errors when callers omit
namespace; add an explicit guard at the start of createPod (in
frontend/e2e/clients/kubernetes-client.ts) that checks pod.metadata and
pod.metadata.namespace and throws a clear Error (e.g., "createPod: missing
pod.metadata.namespace") if absent, before calling
this.k8sApi.createNamespacedPod, so callers get a deterministic validation error
instead of an exception from the K8s client.
In `@frontend/e2e/pages/alertmanager-page.ts`:
- Around line 150-157: yaml.load(yamlContent) can return
undefined/null/primitives so asserting to AlertmanagerConfig is unsafe; add a
runtime guard after calling yaml.load to ensure config is an object before
accessing config.receivers or config.global. Specifically, in the function using
AlertmanagerConfig, check that the result of yaml.load(yamlContent) is a
non-null object (e.g., typeof === "object" && result !== null) and only then
cast to AlertmanagerConfig; fallback to an empty object for config so that the
lookup for receiver (using receiverName and configName) and the returned
globals/receiverConfig are safe (return {} or appropriate defaults when config
is invalid). Ensure the receiver variable lookup (config.receivers?.find(...))
and the return of globals and receiverConfig use this guarded config.
In `@frontend/e2e/pages/navigation.ts`:
- Around line 6-23: Update the Navigation class to extend BasePage and call
super(page) in the constructor, convert ephemeral role-based queries into
private readonly locator properties using this.page.getByTestId(...) (e.g.
private readonly sectionButton = this.page.getByTestId('nav-section-{name}') and
private readonly navLink = this.page.getByTestId('nav-link-{name}') — choose the
app's existing test-id patterns), and refactor navigateViaNav to use those
private locators (lookup the specific section/link test-id using the section and
link parameters, click them, and await load state) so the page object follows
the shared contract (BasePage + private readonly test-id locators + async action
methods).
- Around line 20-21: Replace the unconditional toggle of sectionButton with a
conditional check of its expansion state: locate the code using sectionButton
and this.page.getByRole({ name: link }).click() and instead inspect the
sectionButton's expansion attribute (e.g., getAttribute('aria-expanded') or
evaluate to check a CSS/class state) and only invoke sectionButton.click() if it
is not already expanded; after ensuring the section is open, proceed to await
this.page.getByRole('link', { name: link }).click() and optionally wait for the
link to be visible/enabled before clicking.
In `@frontend/e2e/tests/console/crd-extensions/console-cli-download.spec.ts`:
- Around line 39-52: Replace the manual test.afterEach deletion with the test
cleanup fixture: when creating the ConsoleCLIDownload resource, register it with
the cleanup fixture (e.g., cleanup.add or equivalent) instead of relying on
test.afterEach; remove the try/catch that swallows all errors around
k8sClient.deleteCustomResource and let the cleanup fixture handle removal, and
if you must call k8sClient.deleteCustomResource directly only ignore explicit
not-found errors (check error.code/message) for the resource identified by name
and namespace when invoking
k8sClient.deleteCustomResource('console.openshift.io','v1','',
'consoleclidownloads', name).
In `@frontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.ts`:
- Around line 82-109: Replace the broad try/catch cleanup in test.afterEach by
registering created resources with the provided cleanup fixture instead of
swallowing errors: when creating the Pod (podName), the ConsoleExternalLogLink
instance (resource kind 'consoleexternalloglinks' / variable name), and the
Project (projectName) call the cleanup registration helper (cleanup.register or
similar) immediately after creation so the fixture handles deletion and surfaces
unexpected failures; remove the k8sClient.deletePod,
k8sClient.deleteCustomResource and k8sClient.deleteProject try/catch blocks and
rely on the cleanup fixture to perform and report teardown.
In `@frontend/e2e/tests/console/crd-extensions/console-link.spec.ts`:
- Around line 107-120: The test currently deletes the ConsoleLink in a finally
block which swallows errors; instead register the resource deletion with the
shared cleanup fixture when you create the ConsoleLink so teardown is
deterministic and observable: remove the finally/try-catch block and,
immediately after creating the CR (where name is assigned), call
cleanup.add(async () => await
k8sClient.deleteCustomResource('console.openshift.io','v1','', 'consolelinks',
name)); ensure the cleanup callback does not silently swallow errors (let
Playwright surface failures or log them) and keep the test assertions
independent of cleanup.
In `@frontend/e2e/tests/console/crd-extensions/console-notification.spec.ts`:
- Around line 37-139: Replace the ad-hoc try/finally deletion with the shared
cleanup fixture: after creating the ConsoleNotification via
createCustomResourceViaYaml (inside the "Create ConsoleNotification instance via
YAML editor" step) call the cleanup fixture to register the resource for
teardown (e.g., cleanup.registerResource or similar cleanup API) using the same
identifiers (console.openshift.io, v1, '', 'consolenotifications', and name);
remove the explicit deletion step that calls k8sClient.deleteCustomResource and
delete the finally block that also calls k8sClient.deleteCustomResource so
teardown is handled solely by cleanup.
In `@frontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.ts`:
- Around line 59-139: The test currently creates resources with
k8sClient.createProject(projectName) and performs manual teardown in a
try/finally using k8sClient.deleteCustomResource(...) and
k8sClient.deleteProject(...); move those teardown calls into the shared cleanup
fixture instead of the try/finally: after creating the project and after
creating the ConsoleYAMLSample/Job, register cleanup tasks with the suite's
cleanup fixture (e.g., call the fixture's register/add method to delete the
ConsoleYAMLSample via k8sClient.deleteCustomResource and to delete the project
via k8sClient.deleteProject) and then remove the surrounding try/finally block
and duplicate deletes so the test body only creates/asserts and cleanup handles
all deletions.
In `@frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts`:
- Around line 80-82: jsYaml.load(existingContent) can return undefined or null
so guard before spreading: ensure existingObj is a plain object (fallback to {})
after calling jsYaml.load, then create mergedObj from that safe object and obj;
similarly apply the same guard where jsYaml.load is used at the other location
(lines referenced) so that functions/operations using existingObj, mergedObj, or
passing the value to modifier never receive null/undefined.
In `@frontend/e2e/tests/console/events/events.spec.ts`:
- Around line 44-55: Replace the manual afterEach teardown that calls
k8sClient.deletePod/deleteProject with the test cleanup fixture: when you create
resources (podName/projectName) in the test or its setup, call the cleanup
fixture registration method (e.g., cleanup.registerPod(podName, projectName) or
cleanup.registerProject(projectName) depending on your cleanup API) so the
global cleanup handles deletion; remove the try/catch block in test.afterEach
and ensure all places that call k8sClient.createPod/createProject register those
names with the cleanup fixture instead.
- Around line 79-80: Replace the flaky exact-count assertion on the
'event-warning' test locator with a presence check plus a content check: instead
of asserting toHaveCount(1) on page.getByTestId('event-warning'), assert the
locator's count is >= 1 (e.g., expect(await
page.getByTestId('event-warning').count()).toBeGreaterThanOrEqual(1)) and
additionally assert the warning text contains the expected ImagePullBackOff
message using page.getByTestId('event-warning').toContainText(...) or by
locating the specific text within that locator, referencing the existing
page.getByTestId('event-warning') and page.getByTestId('event-totals') locators.
In `@frontend/e2e/tests/console/favorites/favorites.spec.ts`:
- Around line 55-85: The test "Disable add to favorite button when limit
reached" fills favorites up and never cleans them; update the test to remove the
created entries in a teardown/cleanup block (e.g., finally or an afterEach for
this step) so later specs are not polluted: identify the created favorites by
the names you filled (`test-favorite-${i}`) or by using the same UI controls
(navigate to the pages used in the pages array, open the favorite menu via the
`favorite-button`, locate the created entry in the `dialog`/list and
delete/unfavorite it) or call the app's API to clear those favorites; ensure the
cleanup uses the same selectors (`getByTestId('favorite-button')`,
`getByTestId('input-name')`) and runs even on failures.
In `@frontend/e2e/tests/olm/create-namespace.spec.ts`:
- Around line 15-53: Replace the manual teardown in test.afterEach by
registering the same cleanup steps with the shared cleanup fixture: remove the
test.afterEach block and instead call cleanup.register(...) passing async
callbacks that invoke k8sClient.deleteCustomResource for 'subscriptions' and for
each name returned by k8sClient.listCustomResources('clusterserviceversions'),
and k8sClient.deleteProject(nsName); preserve the existing try/catch semantics
by catching errors inside each registered callback so failures are ignored
during cleanup; ensure you reference the same nsName variable and methods
(k8sClient.deleteCustomResource, k8sClient.listCustomResources,
k8sClient.deleteProject) when adding the cleanup registrations.
---
Nitpick comments:
In `@frontend/e2e/pages/alertmanager-page.ts`:
- Line 39: Replace brittle role/name-based Playwright locators that call
this.page.getByRole('button', { name: 'Copy code to clipboard' }) (and similar
occurrences around the other locators) with page.getByTestId('...') calls; add
matching data-test attributes (e.g., data-test="copy-code-button") to the
corresponding React elements where those buttons/texts are rendered so the new
selectors can target them, update the helper(s) in alertmanager-page.ts to use
page.getByTestId for all detections referenced (including the occurrences around
the other block noted), and ensure test IDs follow the repo convention.
In `@frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts`:
- Around line 13-23: Replace fragile placeholder/role/regex selectors with
test-id based selectors: use page.getByTestId(...) instead of
page.locator('input[placeholder="Filter by name"]') for searchInput, replace the
row match (crdRow) that uses getByRole with a test-id on the row element and
fetch it via getByTestId, and swap the kebabButton lookup to
getByTestId('kebab-button'); update the menu action lookup to use a test-id on
the "View instances" menu item (or use getByTestId for the menu) and click via
that selector. Add the corresponding data-test/data-test-id attributes to the
React components rendering the search input, CRD row, kebab button, and "View
instances" menu item so tests can use page.getByTestId consistently, and apply
the same replacements to the other similar selectors elsewhere in this file.
🪄 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: 2358759e-78c9-49db-af52-824237894660
📒 Files selected for processing (40)
frontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/packages/integration-tests/tests/cluster-settings/alertmanager/alertmanager.cy.tsfrontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/email.cy.tsfrontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/pagerduty.cy.tsfrontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/slack.cy.tsfrontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/webhook.cy.tsfrontend/packages/integration-tests/tests/crd-extensions/console-cli-download.cy.tsfrontend/packages/integration-tests/tests/crd-extensions/console-external-log-link.cy.tsfrontend/packages/integration-tests/tests/crd-extensions/console-link.cy.tsfrontend/packages/integration-tests/tests/crd-extensions/console-notification.cy.tsfrontend/packages/integration-tests/tests/crd-extensions/console-yaml-sample.cy.tsfrontend/packages/integration-tests/tests/events/events.cy.tsfrontend/packages/integration-tests/tests/favorite/favorite-option.cy.tsfrontend/packages/integration-tests/tests/i18n/pseudolocalization.cy.tsfrontend/packages/integration-tests/views/alertmanager.tsfrontend/packages/operator-lifecycle-manager/integration-tests/tests/create-namespace.cy.tsfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/public/components/factory/text-filter.tsx
💤 Files with no reviewable changes (15)
- frontend/packages/operator-lifecycle-manager/integration-tests/tests/create-namespace.cy.ts
- frontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/webhook.cy.ts
- frontend/packages/integration-tests/tests/cluster-settings/alertmanager/alertmanager.cy.ts
- frontend/packages/integration-tests/tests/crd-extensions/console-notification.cy.ts
- frontend/packages/integration-tests/tests/crd-extensions/console-cli-download.cy.ts
- frontend/packages/integration-tests/tests/events/events.cy.ts
- frontend/packages/integration-tests/tests/crd-extensions/console-link.cy.ts
- frontend/packages/integration-tests/tests/i18n/pseudolocalization.cy.ts
- frontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/slack.cy.ts
- frontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/email.cy.ts
- frontend/packages/integration-tests/views/alertmanager.ts
- frontend/packages/integration-tests/tests/crd-extensions/console-yaml-sample.cy.ts
- frontend/packages/integration-tests/tests/crd-extensions/console-external-log-link.cy.ts
- frontend/packages/integration-tests/tests/favorite/favorite-option.cy.ts
- frontend/packages/integration-tests/tests/cluster-settings/alertmanager/receivers/pagerduty.cy.ts
📜 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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsxfrontend/public/components/dashboard/project-dashboard/utilization-card.tsxfrontend/e2e/tests/console/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/public/components/factory/text-filter.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/packages/console-shared/src/components/links/ExternalLink.tsxfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.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/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.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/alertmanager-page.tsfrontend/e2e/pages/navigation.ts
🧠 Learnings (2)
📚 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/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.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/crd-extensions/console-cli-download.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsfrontend/e2e/tests/olm/create-namespace.spec.tsfrontend/e2e/pages/alertmanager-page.tsfrontend/e2e/tests/console/events/events.spec.tsfrontend/e2e/tests/console/crd-extensions/console-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsfrontend/e2e/tests/console/crd-extensions/console-notification.spec.tsfrontend/e2e/clients/kubernetes-client.tsfrontend/e2e/tests/console/i18n/pseudolocalization.spec.tsfrontend/e2e/tests/console/crd-extensions/crd-test-utils.tsfrontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/e2e/pages/navigation.tsfrontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.ts
🪛 ast-grep (0.42.2)
frontend/e2e/pages/alertmanager-page.ts
[warning] 115-115: 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(receiverName)
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)
frontend/e2e/tests/console/crd-extensions/console-link.spec.ts
[warning] 91-91: 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(name)
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)
frontend/e2e/tests/console/crd-extensions/console-notification.spec.ts
[warning] 56-56: 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(name)
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)
[warning] 72-72: 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(name)
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)
frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts
[warning] 16-16: 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(crd, 'i')
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)
[warning] 37-37: 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(crd, 'i')
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)
[warning] 40-40: 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(crd, 'i')
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)
[warning] 146-146: 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(name)
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)
frontend/e2e/tests/console/crd-extensions/console-yaml-sample.spec.ts
[warning] 84-84: 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(name)
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)
frontend/e2e/tests/console/crd-extensions/console-external-log-link.spec.ts
[warning] 173-173: 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(podName)
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)
frontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.ts
[warning] 110-110: 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(receiverName)
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)
🔇 Additional comments (14)
frontend/packages/console-app/src/components/nodes/node-dashboard/UtilizationCard.tsx (1)
54-54: LGTM!frontend/packages/console-shared/src/components/links/ExternalLink.tsx (1)
51-51: LGTM!frontend/packages/console-shared/src/hooks/useCRDAdditionalPrinterColumns.ts (1)
16-19: LGTM!Also applies to: 30-30, 32-32
frontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsx (1)
295-295: LGTM!frontend/public/components/dashboard/project-dashboard/utilization-card.tsx (1)
53-53: LGTM!frontend/public/components/factory/text-filter.tsx (1)
22-22: LGTM!frontend/e2e/clients/kubernetes-client.ts (1)
391-398: LGTM!Also applies to: 484-490, 499-507
frontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.ts (1)
39-47: LGTM!frontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.ts (1)
1-255: LGTM!frontend/e2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.ts (1)
1-223: LGTM!frontend/e2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.ts (1)
1-207: LGTM!frontend/e2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.ts (1)
1-139: LGTM!frontend/e2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.ts (1)
1-87: LGTM!frontend/e2e/tests/console/i18n/pseudolocalization.spec.ts (1)
1-77: LGTM!
| async createPod(pod: k8s.V1Pod): Promise<void> { | ||
| await this.k8sApi.createNamespacedPod({ | ||
| namespace: pod.metadata!.namespace!, | ||
| body: pod, | ||
| }); |
There was a problem hiding this comment.
Guard missing metadata.namespace before creating a Pod.
createPod relies on non-null assertions and can fail unclearly when fixtures omit namespace. Validate input early and throw a deterministic error.
Suggested fix
async createPod(pod: k8s.V1Pod): Promise<void> {
+ const namespace = pod.metadata?.namespace;
+ if (!namespace) {
+ throw new Error('Pod metadata.namespace is required');
+ }
await this.k8sApi.createNamespacedPod({
- namespace: pod.metadata!.namespace!,
+ namespace,
body: pod,
});
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/e2e/clients/kubernetes-client.ts` around lines 492 - 496, createPod
uses non-null assertions on pod.metadata!.namespace! and can throw unclear
runtime errors when callers omit namespace; add an explicit guard at the start
of createPod (in frontend/e2e/clients/kubernetes-client.ts) that checks
pod.metadata and pod.metadata.namespace and throws a clear Error (e.g.,
"createPod: missing pod.metadata.namespace") if absent, before calling
this.k8sApi.createNamespacedPod, so callers get a deterministic validation error
instead of an exception from the K8s client.
| const config: AlertmanagerConfig = yaml.load(yamlContent) as AlertmanagerConfig; | ||
| const receiver: AlertmanagerReceiver | undefined = config.receivers?.find( | ||
| (r) => r.name === receiverName, | ||
| ); | ||
|
|
||
| return { | ||
| globals: config.global || {}, | ||
| receiverConfig: receiver?.[configName]?.[0] || {}, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the file structure and locate the alertmanager page file
find . -name "alertmanager-page.ts" -o -name "alertmanager*.ts" | head -20Repository: openshift/console
Length of output: 269
🏁 Script executed:
#!/bin/bash
# Read the alertmanager-page.ts file around lines 150-157
if [ -f "frontend/e2e/pages/alertmanager-page.ts" ]; then
cat -n frontend/e2e/pages/alertmanager-page.ts | sed -n '140,170p'
fiRepository: openshift/console
Length of output: 727
🏁 Script executed:
#!/bin/bash
# Check the imports and yaml library version
cat -n frontend/e2e/pages/alertmanager-page.ts | head -50Repository: openshift/console
Length of output: 2077
🏁 Script executed:
#!/bin/bash
# Find where yaml is imported from and check package.json for version
find . -name "package.json" | xargs grep -l "yaml" | head -5Repository: openshift/console
Length of output: 213
🏁 Script executed:
#!/bin/bash
# Find where getGlobalsAndReceiverConfig is called
rg "getGlobalsAndReceiverConfig" -A 3 -B 3Repository: openshift/console
Length of output: 15666
🏁 Script executed:
#!/bin/bash
# Check if there are test cases for this function
cat -n frontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager-test-utils.tsRepository: openshift/console
Length of output: 1425
🏁 Script executed:
#!/bin/bash
# Check for similar yaml.load usage patterns in the codebase
rg "yaml\.load" -B 2 -A 2Repository: openshift/console
Length of output: 484
Add runtime guard for YAML parsing to prevent unsafe property access.
yaml.load() can return undefined, null, or primitives rather than an object. The type assertion provides no runtime protection; accessing config.receivers or config.global will throw if parsing fails.
Suggested fix
- const config: AlertmanagerConfig = yaml.load(yamlContent) as AlertmanagerConfig;
+ const parsed = yaml.load(yamlContent);
+ const config: AlertmanagerConfig =
+ parsed && typeof parsed === 'object' ? (parsed as AlertmanagerConfig) : {};
const receiver: AlertmanagerReceiver | undefined = config.receivers?.find(
(r) => r.name === receiverName,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const config: AlertmanagerConfig = yaml.load(yamlContent) as AlertmanagerConfig; | |
| const receiver: AlertmanagerReceiver | undefined = config.receivers?.find( | |
| (r) => r.name === receiverName, | |
| ); | |
| return { | |
| globals: config.global || {}, | |
| receiverConfig: receiver?.[configName]?.[0] || {}, | |
| const parsed = yaml.load(yamlContent); | |
| const config: AlertmanagerConfig = | |
| parsed && typeof parsed === 'object' ? (parsed as AlertmanagerConfig) : {}; | |
| const receiver: AlertmanagerReceiver | undefined = config.receivers?.find( | |
| (r) => r.name === receiverName, | |
| ); | |
| return { | |
| globals: config.global || {}, | |
| receiverConfig: receiver?.[configName]?.[0] || {}, |
🤖 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/alertmanager-page.ts` around lines 150 - 157,
yaml.load(yamlContent) can return undefined/null/primitives so asserting to
AlertmanagerConfig is unsafe; add a runtime guard after calling yaml.load to
ensure config is an object before accessing config.receivers or config.global.
Specifically, in the function using AlertmanagerConfig, check that the result of
yaml.load(yamlContent) is a non-null object (e.g., typeof === "object" && result
!== null) and only then cast to AlertmanagerConfig; fallback to an empty object
for config so that the lookup for receiver (using receiverName and configName)
and the returned globals/receiverConfig are safe (return {} or appropriate
defaults when config is invalid). Ensure the receiver variable lookup
(config.receivers?.find(...)) and the return of globals and receiverConfig use
this guarded config.
| export class Navigation { | ||
| constructor(private page: Page) {} | ||
|
|
||
| /** | ||
| * Navigate using the primary nav by expanding a nav section and clicking a link | ||
| * @param section - The nav section to expand (e.g., "Administration", "Workloads") | ||
| * @param link - The link to click within that section (e.g., "CustomResourceDefinitions", "Pods") | ||
| */ | ||
| async navigateViaNav(section: string, link: string): Promise<void> { | ||
| // Navigate to home first to ensure app is loaded | ||
| await this.page.goto('/'); | ||
| const sectionButton = this.page.getByRole('button', { name: section }); | ||
| await sectionButton.waitFor({ state: 'visible' }); | ||
|
|
||
| await sectionButton.click(); | ||
| await this.page.getByRole('link', { name: link }).click(); | ||
| await this.page.waitForLoadState('domcontentloaded'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Align Navigation with the page-object contract (BasePage + test-id locators).
This helper lives under frontend/e2e/pages but does not follow the shared page-object contract. Please extend BasePage, keep locators as private readonly, and prefer getByTestId selectors.
As per coding guidelines: "frontend/e2e/pages/**/*.ts: Extend BasePage ... Locators are private readonly properties; actions are async methods." and "**/*.{tsx,ts}: Always use page.getByTestId('x') for Playwright 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/navigation.ts` around lines 6 - 23, Update the Navigation
class to extend BasePage and call super(page) in the constructor, convert
ephemeral role-based queries into private readonly locator properties using
this.page.getByTestId(...) (e.g. private readonly sectionButton =
this.page.getByTestId('nav-section-{name}') and private readonly navLink =
this.page.getByTestId('nav-link-{name}') — choose the app's existing test-id
patterns), and refactor navigateViaNav to use those private locators (lookup the
specific section/link test-id using the section and link parameters, click them,
and await load state) so the page object follows the shared contract (BasePage +
private readonly test-id locators + async action methods).
| await sectionButton.click(); | ||
| await this.page.getByRole('link', { name: link }).click(); |
There was a problem hiding this comment.
Avoid unconditional section toggle before clicking nav link.
Always clicking the section button can collapse an already-expanded section and make the link click flaky. Check expansion state first.
Suggested fix
- await sectionButton.click();
+ const isExpanded = (await sectionButton.getAttribute('aria-expanded')) === 'true';
+ if (!isExpanded) {
+ await sectionButton.click();
+ }
await this.page.getByRole('link', { name: link }).click();🤖 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/navigation.ts` around lines 20 - 21, Replace the
unconditional toggle of sectionButton with a conditional check of its expansion
state: locate the code using sectionButton and this.page.getByRole({ name: link
}).click() and instead inspect the sectionButton's expansion attribute (e.g.,
getAttribute('aria-expanded') or evaluate to check a CSS/class state) and only
invoke sectionButton.click() if it is not already expanded; after ensuring the
section is open, proceed to await this.page.getByRole('link', { name: link
}).click() and optionally wait for the link to be visible/enabled before
clicking.
| test.afterEach(async () => { | ||
| // Clean up the ConsoleCLIDownload instance | ||
| try { | ||
| await k8sClient.deleteCustomResource( | ||
| 'console.openshift.io', | ||
| 'v1', | ||
| '', | ||
| 'consoleclidownloads', | ||
| name, | ||
| ); | ||
| } catch (error) { | ||
| // Ignore if already deleted | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Use the cleanup fixture instead of manual afterEach deletion.
The current cleanup path swallows all deletion errors, which can mask leaked resources and increase CI flakiness. Please register created resources with the cleanup fixture and only ignore explicit not-found cases when unavoidable.
As per coding guidelines: "Each Playwright E2E test block must create its own resources, assert independently, and clean up via the cleanup fixture".
🤖 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/crd-extensions/console-cli-download.spec.ts`
around lines 39 - 52, Replace the manual test.afterEach deletion with the test
cleanup fixture: when creating the ConsoleCLIDownload resource, register it with
the cleanup fixture (e.g., cleanup.add or equivalent) instead of relying on
test.afterEach; remove the try/catch that swallows all errors around
k8sClient.deleteCustomResource and let the cleanup fixture handle removal, and
if you must call k8sClient.deleteCustomResource directly only ignore explicit
not-found errors (check error.code/message) for the resource identified by name
and namespace when invoking
k8sClient.deleteCustomResource('console.openshift.io','v1','',
'consoleclidownloads', name).
| const existingObj = jsYaml.load(existingContent) as Record<string, any>; | ||
| const mergedObj = { ...existingObj, ...obj }; | ||
| const newYaml = jsYaml.dump(mergedObj, { sortKeys: true }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's read the file to see the context around lines 80-82 and 122-124
cat -n frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts | sed -n '70,130p'Repository: openshift/console
Length of output: 2453
🏁 Script executed (no clone):
#!/bin/bash
# Check js-yaml documentation and behavior for edge cases
# Let's look at what js-yaml.load returns for various inputs
python3 << 'EOF'
import json
# js-yaml behavior documentation:
# According to js-yaml docs, load() returns:
# - The parsed value (object, array, string, number, etc.)
# - undefined if the input is empty or whitespace-only
# - null for explicit null values in YAML
# Test cases:
test_cases = {
"Empty string": {
"input": '""',
"js_yaml_result": "undefined (empty input)",
"spread_behavior": "Error: Cannot spread undefined"
},
"Whitespace only": {
"input": '" "',
"js_yaml_result": "undefined",
"spread_behavior": "Error: Cannot spread undefined"
},
"Valid YAML object": {
"input": '"{}"',
"js_yaml_result": "{}",
"spread_behavior": "OK"
},
"Valid YAML null": {
"input": '"null"',
"js_yaml_result": "null",
"spread_behavior": "TypeError: Cannot spread null"
},
"Malformed YAML": {
"input": '"{: invalid}"',
"js_yaml_result": "throws Error",
"spread_behavior": "Never reached"
}
}
for case, details in test_cases.items():
print(f"{case}:")
for key, value in details.items():
print(f" {key}: {value}")
EOF
</parameter>
Length of output: 668
🏁 Script executed (no clone):
#!/bin/bash
# Check the actual js-yaml package to understand return behavior
# js-yaml is a common YAML parser for JavaScript
cat << 'EOF'
According to js-yaml documentation (https://github.com/nodeca/js-yaml):
jsYaml.load(str, options) returns:
- Parsed value for valid YAML
- undefined if input is empty/whitespace-only
- null for YAML null values
- Throws error for invalid YAML (unless options.strict: false)
Critical issue with spreading:
- Spreading undefined: { ...undefined, ...obj } => TypeError
- Spreading null: { ...null, ...obj } => TypeError
- Both need to be guarded
The review comment is CORRECT that jsYaml.load() can return undefined.
EOF
</parameter>
Length of output: 674
🏁 Script executed:
#!/bin/bash
# Read the getYamlEditorContent function to understand context
cat -n frontend/e2e/tests/console/crd-extensions/crd-test-utils.ts | sed -n '40,80p'Repository: openshift/console
Length of output: 1697
Guard against undefined or null from jsYaml.load() before spreading/using.
jsYaml.load() returns undefined for empty/whitespace input and null for explicit YAML nulls. Spreading or passing these to the modifier function will throw. Fallback to an empty object.
Suggested fix
export async function mergeYamlEditorContent(page: Page, obj: any): Promise<void> {
const existingContent = await getYamlEditorContent(page);
- const existingObj = jsYaml.load(existingContent) as Record<string, any>;
+ const parsed = jsYaml.load(existingContent);
+ const existingObj =
+ parsed && typeof parsed === 'object' ? (parsed as Record<string, any>) : {};
const mergedObj = { ...existingObj, ...obj };
const newYaml = jsYaml.dump(mergedObj, { sortKeys: true });
await setYamlEditorContent(page, newYaml);
}
@@
const existingContent = await getYamlEditorContent(page);
- const existingObj = jsYaml.load(existingContent) as Record<string, any>;
+ const parsed = jsYaml.load(existingContent);
+ const existingObj =
+ parsed && typeof parsed === 'object' ? (parsed as Record<string, any>) : {};
const modifiedObj = modifier(existingObj);Also applies to: lines 122–124
🤖 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/crd-extensions/crd-test-utils.ts` around lines 80
- 82, jsYaml.load(existingContent) can return undefined or null so guard before
spreading: ensure existingObj is a plain object (fallback to {}) after calling
jsYaml.load, then create mergedObj from that safe object and obj; similarly
apply the same guard where jsYaml.load is used at the other location (lines
referenced) so that functions/operations using existingObj, mergedObj, or
passing the value to modifier never receive null/undefined.
| test.afterEach(async () => { | ||
| try { | ||
| await k8sClient.deletePod(podName, projectName); | ||
| } catch { | ||
| // Ignore if already deleted | ||
| } | ||
| try { | ||
| await k8sClient.deleteProject(projectName); | ||
| } catch { | ||
| // Ignore if already deleted | ||
| } | ||
| }); |
There was a problem hiding this comment.
Prefer cleanup fixture over manual afterEach teardown.
Line 44–Line 55 performs manual resource deletion. Please register created resources in the cleanup fixture to align with required E2E isolation/teardown behavior.
As per coding guidelines, “Each Playwright E2E test block must create its own resources, assert independently, and clean up via the cleanup fixture”.
🤖 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/events/events.spec.ts` around lines 44 - 55,
Replace the manual afterEach teardown that calls
k8sClient.deletePod/deleteProject with the test cleanup fixture: when you create
resources (podName/projectName) in the test or its setup, call the cleanup
fixture registration method (e.g., cleanup.registerPod(podName, projectName) or
cleanup.registerProject(projectName) depending on your cleanup API) so the
global cleanup handles deletion; remove the try/catch block in test.afterEach
and ensure all places that call k8sClient.createPod/createProject register those
names with the cleanup fixture instead.
| await expect(page.getByTestId('event-totals')).toContainText('1 event'); | ||
| await expect(page.getByTestId('event-warning')).toHaveCount(1); |
There was a problem hiding this comment.
Avoid exact event count assertions in this flow.
Line 79–Line 80 expects exactly one warning event, but ImagePullBackOff flows typically emit repeated warnings and can make this flaky. Assert presence (>= 1) plus the target message instead of exact cardinality.
Suggested stabilization
- await expect(page.getByTestId('event-totals')).toContainText('1 event');
- await expect(page.getByTestId('event-warning')).toHaveCount(1);
+ await expect(page.getByTestId('event-totals')).toContainText('event');
+ expect(await page.getByTestId('event-warning').count()).toBeGreaterThan(0);
+ await expect(page.getByText('Error: ImagePullBackOff')).toBeVisible();🤖 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/events/events.spec.ts` around lines 79 - 80,
Replace the flaky exact-count assertion on the 'event-warning' test locator with
a presence check plus a content check: instead of asserting toHaveCount(1) on
page.getByTestId('event-warning'), assert the locator's count is >= 1 (e.g.,
expect(await
page.getByTestId('event-warning').count()).toBeGreaterThanOrEqual(1)) and
additionally assert the warning text contains the expected ImagePullBackOff
message using page.getByTestId('event-warning').toContainText(...) or by
locating the specific text within that locator, referencing the existing
page.getByTestId('event-warning') and page.getByTestId('event-totals') locators.
| await test.step('Disable add to favorite button when limit reached', async () => { | ||
| const pages = [ | ||
| '/', | ||
| '/k8s/all-namespaces/core~v1~Pod', | ||
| '/k8s/all-namespaces/apps~v1~Deployment', | ||
| '/k8s/all-namespaces/core~v1~Secret', | ||
| '/k8s/all-namespaces/core~v1~ConfigMap', | ||
| '/k8s/cluster/core~v1~Node', | ||
| '/k8s/all-namespaces/batch~v1~CronJob', | ||
| '/k8s/all-namespaces/batch~v1~Job', | ||
| '/k8s/all-namespaces/apps~v1~ReplicaSet', | ||
| '/k8s/all-namespaces/core~v1~ReplicationController', | ||
| ]; | ||
|
|
||
| for (let i = 0; i < pages.length; i++) { | ||
| await page.goto(pages[i]); | ||
| await page.getByTestId('favorite-button').first().waitFor({ state: 'visible' }); | ||
| await page.getByTestId('favorite-button').first().click(); | ||
| const dialog = page.getByRole('dialog'); | ||
| await expect(dialog).toContainText('Add to favorites'); | ||
| const nameInput = page.getByTestId('input-name'); | ||
| await nameInput.clear(); | ||
| await nameInput.fill(`test-favorite-${i}`); | ||
| await nameInput.press('Enter'); | ||
| await expect(dialog).toBeHidden(); | ||
| } | ||
|
|
||
| await page.goto('/k8s/all-namespaces/apps~v1~DaemonSet'); | ||
| await page.getByTestId('favorite-button').first().waitFor({ state: 'visible' }); | ||
| await expect(page.getByTestId('favorite-button').first()).toBeDisabled(); | ||
| }); |
There was a problem hiding this comment.
This test leaves favorites state polluted for subsequent tests.
Line 55–Line 85 fills favorites to the limit and does not reset them. If favorites are persisted for the test user, this can cause cross-test contamination/flakes in later specs. Please clear entries created in this test during teardown.
🤖 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/favorites/favorites.spec.ts` around lines 55 - 85,
The test "Disable add to favorite button when limit reached" fills favorites up
and never cleans them; update the test to remove the created entries in a
teardown/cleanup block (e.g., finally or an afterEach for this step) so later
specs are not polluted: identify the created favorites by the names you filled
(`test-favorite-${i}`) or by using the same UI controls (navigate to the pages
used in the pages array, open the favorite menu via the `favorite-button`,
locate the created entry in the `dialog`/list and delete/unfavorite it) or call
the app's API to clear those favorites; ensure the cleanup uses the same
selectors (`getByTestId('favorite-button')`, `getByTestId('input-name')`) and
runs even on failures.
| test.afterEach(async () => { | ||
| try { | ||
| await k8sClient.deleteCustomResource( | ||
| 'operators.coreos.com', | ||
| 'v1alpha1', | ||
| nsName, | ||
| 'subscriptions', | ||
| '3scale-community-operator', | ||
| ); | ||
| } catch { | ||
| // Ignore if not created | ||
| } | ||
| try { | ||
| const csvs = (await k8sClient.listCustomResources( | ||
| 'operators.coreos.com', | ||
| 'v1alpha1', | ||
| nsName, | ||
| 'clusterserviceversions', | ||
| )) as Array<{ metadata?: { name?: string } }>; | ||
| for (const csv of csvs) { | ||
| if (csv.metadata?.name) { | ||
| await k8sClient.deleteCustomResource( | ||
| 'operators.coreos.com', | ||
| 'v1alpha1', | ||
| nsName, | ||
| 'clusterserviceversions', | ||
| csv.metadata.name, | ||
| ); | ||
| } | ||
| } | ||
| } catch { | ||
| // Ignore cleanup errors | ||
| } | ||
| try { | ||
| await k8sClient.deleteProject(nsName); | ||
| } catch { | ||
| // Ignore if not created | ||
| } | ||
| }); |
There was a problem hiding this comment.
Adopt cleanup fixture for teardown instead of manual afterEach cleanup.
Line 15–Line 53 manually removes subscription/CSV/project. Please move teardown registration to the shared cleanup fixture to match the required Playwright E2E lifecycle and reduce drift across suites.
As per coding guidelines, “Each Playwright E2E test block must create its own resources, assert independently, and clean up via the cleanup fixture”.
🤖 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/olm/create-namespace.spec.ts` around lines 15 - 53,
Replace the manual teardown in test.afterEach by registering the same cleanup
steps with the shared cleanup fixture: remove the test.afterEach block and
instead call cleanup.register(...) passing async callbacks that invoke
k8sClient.deleteCustomResource for 'subscriptions' and for each name returned by
k8sClient.listCustomResources('clusterserviceversions'), and
k8sClient.deleteProject(nsName); preserve the existing try/catch semantics by
catching errors inside each registered callback so failures are ignored during
cleanup; ensure you reference the same nsName variable and methods
(k8sClient.deleteCustomResource, k8sClient.listCustomResources,
k8sClient.deleteProject) when adding the cleanup registrations.
…anager, and OLM Cypress tests to Playwright Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CodeRabbit review — triageFixed (3)
Skipped
|
|
@rhamilto: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| import { coFetchJSON } from '@console/shared/src/utils/console-fetch'; | ||
|
|
||
| export const useCRDAdditionalPrinterColumns = ( | ||
| model: K8sModel, |
There was a problem hiding this comment.
| model: K8sModel | undefined, |
3 participants
Includes changes from #16444
Summary
alertmanager.tsview filedata-testattributes alongside existingdata-test-idattributes on utilization cards, text filter, and external links to enablegetByTestIdusagecreatePodanddeletePodmethods toKubernetesClientAlertmanagerPagepage object and navigation helperswaitForTimeoutornetworkidleanti-patterns)Tests migrated (19 Cypress → 19 Playwright)
integration-tests/tests/crd-extensions/console-cli-download.cy.tse2e/tests/console/crd-extensions/console-cli-download.spec.tsintegration-tests/tests/crd-extensions/console-external-log-link.cy.tse2e/tests/console/crd-extensions/console-external-log-link.spec.tsintegration-tests/tests/crd-extensions/console-link.cy.tse2e/tests/console/crd-extensions/console-link.spec.tsintegration-tests/tests/crd-extensions/console-notification.cy.tse2e/tests/console/crd-extensions/console-notification.spec.tsintegration-tests/tests/crd-extensions/console-yaml-sample.cy.tse2e/tests/console/crd-extensions/console-yaml-sample.spec.tsintegration-tests/tests/events/events.cy.tse2e/tests/console/events/events.spec.tsintegration-tests/tests/favorite/favorite-option.cy.tse2e/tests/console/favorites/favorites.spec.tsintegration-tests/tests/i18n/pseudolocalization.cy.tse2e/tests/console/i18n/pseudolocalization.spec.tsolm/integration-tests/tests/create-namespace.cy.tse2e/tests/olm/create-namespace.spec.tsintegration-tests/tests/cluster-settings/alertmanager/alertmanager.cy.tse2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsintegration-tests/tests/cluster-settings/alertmanager/receivers/email.cy.tse2e/tests/console/cluster-settings/alertmanager/receivers/email.spec.tsintegration-tests/tests/cluster-settings/alertmanager/receivers/pagerduty.cy.tse2e/tests/console/cluster-settings/alertmanager/receivers/pagerduty.spec.tsintegration-tests/tests/cluster-settings/alertmanager/receivers/slack.cy.tse2e/tests/console/cluster-settings/alertmanager/receivers/slack.spec.tsintegration-tests/tests/cluster-settings/alertmanager/receivers/webhook.cy.tse2e/tests/console/cluster-settings/alertmanager/receivers/webhook.spec.tsCypress cleanup
integration-tests/views/alertmanager.ts(only consumed by the migrated alertmanager tests)Source component changes
text-filter.tsx: Addeddata-test="item-filter"alongsidedata-test-idutilization-card.tsx(3 files): Addeddata-test="utilization-card"alongsidedata-test-iduseCRDAdditionalPrinterColumns.ts: Fixedreact-hooks/exhaustive-depslint warningExternalLink.tsx: Addeddata-testattributeScreenshots / Screen recordings
Automated test migration — no visual changes
Test plan
cy.wait(10000))🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes