CONSOLE-5196: Fix Playwright e2e flakes across multiple test suites - #16881
Open
rhamilto wants to merge 6 commits into
Open
CONSOLE-5196: Fix Playwright e2e flakes across multiple test suites#16881rhamilto wants to merge 6 commits into
rhamilto wants to merge 6 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>
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@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. |
Contributor
|
[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
Contributor
WalkthroughChangesThe E2E suite adds Alertmanager receiver coverage and updates SPA setup, perspective configuration, route readiness, network synchronization, cleanup, timeouts, topology form handling, and accessibility loading detection. ChangesE2E validation and reliability
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Member
Author
|
/test e2e-playwright |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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>
Member
Author
|
/test e2e-playwright |
1 task
Member
Author
|
/test e2e-playwright |
Member
Author
|
/test e2e-playwright |
logonoff
reviewed
Jul 30, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
Fixed. Thanks, @logonoff. |
Member
Author
|
/test e2e-playwright |
1 similar comment
Member
Author
|
/test e2e-playwright |
…is unavailable warmupSPA waited for a page-heading element that doesn't exist on the Topology page, causing timeouts after OAuth redirects restored a prior route. Switch to #page-sidebar which is present on every console page. Extract ensureDeveloperPerspective helper that patches the Console operator config to enable the Developer perspective when the cluster only exposes a single perspective (OCP 5.0 default). Use it in search, configure-perspectives, and cluster-customization specs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reverts an earlier change that switched from getByTestId to
locator('[data-test-id=...]'). Since testIdAttribute is configured
as data-test, getByTestId is the idiomatic Playwright approach and
matches ensureDeveloperPerspective.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/base-page.ts`:
- Around line 43-55: Update ensureDeveloperPerspective in the Console/cluster
setup flow to capture the existing spec.customization.perspectives before
patching it, then restore that exact configuration during suite teardown. Ensure
restoration is registered for all invocation paths and still runs when setup or
tests fail, preserving the shared cluster resource for later suites.
🪄 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: a4ff8292-7433-4160-ba85-d884da381e82
📒 Files selected for processing (4)
frontend/e2e/pages/base-page.tsfrontend/e2e/tests/dev-console/cluster-customization.spec.tsfrontend/e2e/tests/dev-console/configure-perspectives.spec.tsfrontend/e2e/tests/dev-console/search.spec.ts
Comment on lines
+43
to
+55
| await k8sClient.customObjectsApi.patchClusterCustomObject({ | ||
| group: 'operator.openshift.io', | ||
| version: 'v1', | ||
| plural: 'consoles', | ||
| name: 'cluster', | ||
| body: [ | ||
| { | ||
| op: 'add', | ||
| path: '/spec/customization/perspectives', | ||
| value: [{ id: 'dev', visibility: { state: 'Enabled' } }], | ||
| }, | ||
| ], | ||
| }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline frontend/e2e/pages/base-page.ts --match ensureDeveloperPerspective --view expanded
rg -n -C 8 \
'ensureDeveloperPerspective\s*\(|patchClusterCustomObject|customization/perspectives|afterAll|afterEach' \
frontend/e2eRepository: openshift/console
Length of output: 49633
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- base-page.ts ---'
cat -n frontend/e2e/pages/base-page.ts | sed -n '1,80p'
printf '%s\n' '--- all ensureDeveloperPerspective call sites ---'
rg -l 'ensureDeveloperPerspective\s*\(' frontend/e2e --glob '*.ts' | sort | while read -r file; do
printf '\n## %s\n' "$file"
rg -n -C 12 'ensureDeveloperPerspective\s*\(' "$file"
done
printf '%s\n' '--- Kubernetes client read and patch APIs ---'
rg -n -C 6 \
'readClusterCustomObject|patchClusterCustomObject|customObjectsApi' \
frontend/e2e/fixtures frontend/e2e/pages frontend/e2e/tests --glob '*.ts' | head -n 400
printf '%s\n' '--- fixture and Playwright configuration ---'
fd -i -t f 'fixture|playwright|config' frontend/e2e . | sort | head -n 120
rg -n -C 8 \
'k8sClient|test\.extend|fullyParallel|workers|forbidOnly|afterAll|afterEach' \
frontend/e2e --glob '*.{ts,js}' | head -n 500Repository: openshift/console
Length of output: 50373
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path("frontend/e2e")
pattern = re.compile(r"ensureDeveloperPerspective\s*\(")
for path in sorted(root.rglob("*.ts")):
text = path.read_text(errors="replace")
matches = list(pattern.finditer(text))
if not matches:
continue
print(f"{path}: {len(matches)} occurrence(s)")
for m in matches:
line = text.count("\n", 0, m.start()) + 1
start = max(0, m.start() - 500)
end = min(len(text), m.end() + 500)
context = text[start:end].replace("\n", " ")
print(f" line {line}: {context}")
PYRepository: openshift/console
Length of output: 4140
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- e2e files relevant to fixture scope and runner settings ---'
git ls-files frontend/e2e | grep -Ei '(^|/)(fixtures?/index|playwright.*config|.*config.*playwright|kubernetes-client)\.(ts|js|mjs|cjs)$' | sort
printf '%s\n' '--- fixture definition ---'
cat -n frontend/e2e/fixtures/index.ts | sed -n '1,120p'
printf '%s\n' '--- runner configuration candidates ---'
for file in $(git ls-files | grep -Ei '(^|/)(playwright.*config|.*config.*playwright)\.(ts|js|mjs|cjs)$' | head -n 20); do
printf '\n## %s\n' "$file"
cat -n "$file" | sed -n '1,180p'
done
printf '%s\n' '--- KubernetesClient API declarations and custom-object methods ---'
client_files=$(git ls-files frontend/e2e | grep -E '/kubernetes-client\.(ts|js)$' || true)
for file in $client_files; do
printf '\n## %s\n' "$file"
rg -n -C 5 \
'class KubernetesClient|getClusterCustomObject|patchClusterCustomObject|customObjectsApi|constructor' \
"$file"
doneRepository: openshift/console
Length of output: 10107
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path("frontend/e2e/tests")
call = re.compile(r"ensureDeveloperPerspective\s*\(")
for path in sorted(root.rglob("*.ts")):
text = path.read_text(errors="replace")
for match in call.finditer(text):
line = text.count("\n", 0, match.start()) + 1
before = text[:match.start()]
hooks = [(m.start(), m.group(0), before.count("\n", 0, m.start()) + 1)
for m in re.finditer(r"test\.(beforeAll|beforeEach|afterAll|afterEach)\s*\(", before)]
nearest = hooks[-1] if hooks else None
print(f"{path}:{line}: nearest preceding hook={nearest}")
PYRepository: openshift/console
Length of output: 467
Restore the original Console perspective configuration.
When ensureDeveloperPerspective changes Console/cluster, preserve spec.customization.perspectives and restore it in suite teardown, including failure paths. The helper runs from multiple test hooks and can leave the shared cluster resource modified for later suites.
🤖 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/base-page.ts` around lines 43 - 55, Update
ensureDeveloperPerspective in the Console/cluster setup flow to capture the
existing spec.customization.perspectives before patching it, then restore that
exact configuration during suite teardown. Ensure restoration is registered for
all invocation paths and still runs when setup or tests fail, preserving the
shared cluster resource for later suites.
Member
Author
|
/test e2e-playwright |
Contributor
|
@rhamilto: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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('/')waits forpage-heading, but after OAuth redirect the SPA can restore to any previous route (e.g. Topology), which has nopage-headingelement. This causes 30s timeout failures.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.search.spec.ts,configure-perspectives.spec.ts, andcluster-customization.spec.tscallswitchPerspective('Developer')or expect a Developer tab without first ensuring the perspective is available, causing failures on CI clusters.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. Wait for#page-sidebarinstead ofpage-heading— the sidebar is present on every console page regardless of perspective or route.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.ensureDeveloperPerspectivehelper inbase-page.tsthat patches the Console operator config to enable the Developer perspective when the cluster only exposes a single perspective. Use it insearch.spec.ts,configure-perspectives.spec.ts, andcluster-customization.spec.tsviabeforeEach. Also alignswitchPerspectiveto usegetByTestIdconsistently.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— 3/3 passed (warmupSPA + perspective enablement)configure-perspectives.spec.ts— 1/1 passed (perspective enablement)cluster-customization.spec.ts— 7/7 passed (perspective enablement)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
Tests
Bug Fixes / Reliability
Accessibility