CONSOLE-5196: Fix Playwright e2e flakes across multiple test suites - #16881
CONSOLE-5196: Fix Playwright e2e flakes across multiple test suites#16881rhamilto wants to merge 3 commits into
Conversation
Address several sources of flaky Playwright e2e test failures: 1. Alertmanager parallel worker interference: Merge receivers.spec.ts into alertmanager.spec.ts so all tests that mutate the shared alertmanager-main secret run on a single worker. 2. warmupSPA resilience: Wrap the goto + heading check in toPass() with retry intervals so a single slow cluster response doesn't cause immediate failure. 3. a11y test stabilization: Wait for loading indicators to clear before running axe-core analysis. Upgrade toBeAttached() checks to toBeVisible() in assertLoaded callbacks so pages are fully rendered before a11y checks run. 4. Topology test timeouts: Add test.setTimeout(300_000) to tests that create workloads via git URL, since validation + deployment regularly exceeds the 120s default. 5. Topology application name field: Handle the conditional rendering of the application name input by selecting "Create application" from the dropdown when existing applications are present. 6. Perspective-switcher cleanup: Add afterAll to remove the Console operator perspectives patch, preventing cluster-wide side effects from leaking into other tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@rhamilto: This pull request references CONSOLE-5196 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 epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is 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
the
approved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChangesThe E2E suite now retries SPA warmup, waits for visible UI and loading states, handles optional topology application selection, synchronizes console update checks, cleans up perspective patches, extends selected topology timeouts, and adds Alertmanager receiver form coverage. E2E readiness and synchronization
Alertmanager receiver validation
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/test e2e-playwright |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/e2e/tests/console/crud/other-routes.spec.ts (1)
155-169: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider logging cleanup patch failures instead of silently swallowing.
If the
removepatch for/spec/customization/perspectivesfails, the error is discarded via.catch(() => {}), leaving the customization on the cluster with no trace in CI logs — this can silently pollute state for later test runs.♻️ Log cleanup failures for visibility
await k8sClient.customObjectsApi .patchClusterCustomObject({ group: 'operator.openshift.io', version: 'v1', plural: 'consoles', name: 'cluster', body: [{ op: 'remove', path: '/spec/customization/perspectives' }], }) - .catch(() => {}); + .catch((error) => { + console.error('[Cleanup] Failed to remove perspectives customization:', error); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/tests/console/crud/other-routes.spec.ts` around lines 155 - 169, Update the cleanup logic in the test.afterAll handler for patchedPerspectives so failures from patchClusterCustomObject are logged instead of being silently discarded by catch. Preserve the existing cleanup attempt and patch payload, and include the caught error in the CI-visible log output.frontend/e2e/utils/a11y.ts (1)
9-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate loading-indicator selector list across two files. The same CSS selector list is maintained independently in
a11y.tsandbase-page.ts, risking drift as new loading/skeleton patterns are added.
frontend/e2e/utils/a11y.ts#L9-L19: extractLOADING_SELECTORSto a shared module (e.g., re-export from or importBasePage's selector list) instead of redeclaring it.frontend/e2e/pages/base-page.ts#L35-L45: source the shared selector list from the same module so both files stay in sync.🤖 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/utils/a11y.ts` around lines 9 - 19, Eliminate the duplicated loading selector list by defining one shared LOADING_SELECTORS source and importing or re-exporting it wherever needed. Update frontend/e2e/utils/a11y.ts lines 9-19 to consume the shared list, and update frontend/e2e/pages/base-page.ts lines 35-45 to consume that same source instead of redeclaring it; preserve the existing selector values and behavior.
🤖 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/topology-page.ts`:
- Around line 197-201: Update the applicationDropdown handling in the topology
page flow to wait for visibility with a real 2-second wait, replacing the
timeout-bearing isVisible check. Preserve the fallback behavior when the
dropdown does not appear, then click the dropdown and the “Create application”
option only after the visibility wait succeeds.
---
Nitpick comments:
In `@frontend/e2e/tests/console/crud/other-routes.spec.ts`:
- Around line 155-169: Update the cleanup logic in the test.afterAll handler for
patchedPerspectives so failures from patchClusterCustomObject are logged instead
of being silently discarded by catch. Preserve the existing cleanup attempt and
patch payload, and include the caught error in the CI-visible log output.
In `@frontend/e2e/utils/a11y.ts`:
- Around line 9-19: Eliminate the duplicated loading selector list by defining
one shared LOADING_SELECTORS source and importing or re-exporting it wherever
needed. Update frontend/e2e/utils/a11y.ts lines 9-19 to consume the shared list,
and update frontend/e2e/pages/base-page.ts lines 35-45 to consume that same
source instead of redeclaring it; preserve the existing selector values and
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a4a114a1-842b-4cb0-98ff-858775c7d03d
📒 Files selected for processing (7)
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/topology-page.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/alertmanager.spec.tsfrontend/e2e/tests/console/cluster-settings/alertmanager/receivers/receivers.spec.tsfrontend/e2e/tests/console/crud/other-routes.spec.tsfrontend/e2e/tests/topology/topology-ci.spec.tsfrontend/e2e/utils/a11y.ts
💤 Files with no reviewable changes (1)
- frontend/e2e/tests/console/cluster-settings/alertmanager/receivers/receivers.spec.ts
After swapping the route handler, explicitly wait for the next check-updates response before asserting the toast. This eliminates the race where the 15s polling interval drifts under CI load and the toast assertion times out before the component receives the new data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
locator.isVisible() returns immediately and ignores the timeout
parameter. Use waitFor({ state: 'visible' }) to actually wait up
to 2s for the application dropdown to render.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test e2e-playwright |
|
@rhamilto: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/test e2e-playwright |
2 participants
Analysis / Root cause:
Multiple pre-existing flaky Playwright e2e tests are causing CI failures across PRs (e.g. #16658). Follow-on to #16863 which addressed some of these. Root causes:
alertmanager.spec.tsandreceivers/receivers.spec.tsboth mutate the sharedalertmanager-mainsecret viaresetAlertmanagerConfig()in beforeEach/afterEach. WithWORKERS=2, these files land on different workers and clobber each other's config.page.goto('/')+ heading visibility check with no retry. On slow CI clusters, this regularly exceeds the combined 90s timeout.testA11yruns axe-core immediately afterassertLoaded, but someassertLoadedcallbacks only checktoBeAttached()(in DOM, not necessarily rendered). Axe finds false-positive violations in partially rendered DOM.fillApplicationNameassumes the text input is always visible, but it's conditionally rendered only when no applications exist or "Create application" is selected. When prior tests leave application groups in the namespace, the dropdown shows instead.Solution description:
receivers/receivers.spec.tsintoalertmanager.spec.tsso all alertmanager tests share a single worker via the file-leveltest.describe.configure({ mode: 'serial' }).warmupSPAinexpect().toPass()with escalating retry intervals (1s, 2s, 5s) and 90s overall timeout.testA11ybefore running axe-core. UpgradetoBeAttached()totoBeVisible()inassertLoadedcallbacks for routes/,/k8s/cluster/clusterroles/view,/api-resource/.../schema, and/settings/cluster.test.setTimeout(300_000)to the 4 topology tests that create and delete workloads.fillApplicationNameto detect when the application dropdown is visible and select "Create application" before filling the text input.afterAllcleanup to remove the Console operatorperspectivespatch when it was applied.Screenshots / screen recording:
N/A — test infrastructure only, no UI changes.
Test setup:
None required.
Test cases:
All affected test suites pass locally against a live cluster:
alertmanager.spec.ts— 14/14 passed (merged file)search.spec.ts— 7/7 passed (warmupSPA retry)other-routes.spec.ts— 28/28 passed (a11y + perspective cleanup)topology-ci.spec.ts— 9/9 passed (timeout + application name fix)Browser conformance:
Additional info:
Continues the flake-fixing work from #16863. Should unblock PRs like #16658 that are stuck on pre-existing CI flakes.
Reviewers and assignees:
Summary by CodeRabbit