CONSOLE-5196: Fix Playwright CI timeout with globalTimeout and parallel workers#16780
CONSOLE-5196: Fix Playwright CI timeout with globalTimeout and parallel workers#16780rhamilto wants to merge 3 commits into
Conversation
…el workers The Playwright e2e suite was exceeding Prow's 2-hour timeout with 299+ tests running on a single worker. The container was killed mid-run (exit code 127), preventing clean JUnit output and artifact collection. Add a 110-minute globalTimeout so Playwright exits cleanly before Prow kills the process, and default to 2 workers in the Prow entrypoint to cut wall-clock time. Both values are env-var overridable. 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. |
|
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 (2)
WalkthroughPlaywright now supports environment-configured global timeouts and worker counts. E2E tests add polling synchronization, stable readiness checks, and persisted Favorites-state cleanup. ChangesE2E reliability updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/playwright.config.ts`:
- Line 52: Update the globalTimeout configuration expression to distinguish an
explicitly provided GLOBAL_TIMEOUT_MS value of "0" from an unset or invalid
value. Use the parsed environment value whenever GLOBAL_TIMEOUT_MS is explicitly
present, and only apply the isCI-based 110-minute default when the variable is
absent.
- Line 88: Update the workers configuration to preserve valid WORKERS values,
including Playwright percentage strings, instead of coercing them with parseInt.
Validate the environment value before use and pass the original string through
unchanged when valid, while retaining undefined when WORKERS is unset and
rejecting malformed values such as “2foo”.
🪄 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: 96cb9129-8fef-4082-aaa6-786ec342575a
📒 Files selected for processing (2)
frontend/playwright.config.tstest-prow-playwright-e2e.sh
| testDir: './e2e/tests', | ||
| testMatch: '**/*.spec.ts', | ||
| forbidOnly: isCI, | ||
| globalTimeout: parseInt(process.env.GLOBAL_TIMEOUT_MS || '0', 10) || (isCI ? 110 * 60 * 1000 : 0), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant config file with line numbers.
sed -n '1,160p' frontend/playwright.config.ts | cat -nRepository: openshift/console
Length of output: 5460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,160p' frontend/playwright.config.ts | cat -nRepository: openshift/console
Length of output: 5460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pwd
lsRepository: openshift/console
Length of output: 844
🌐 Web query:
Playwright globalTimeout 0 meaning disable timeout documentation
💡 Result:
In Playwright, setting globalTimeout to 0 means that the timeout behavior is disabled, allowing the test suite to run for an unlimited amount of time [1][2]. The globalTimeout configuration option defines the maximum time in milliseconds that the entire test suite is allowed to run [1]. By default, there is no global timeout (i.e., it is unlimited) [3][4]. When you explicitly set globalTimeout to 0, you are effectively choosing to disable any automatic termination of the test run based on duration, which can be useful if you do not want Playwright to stop the process prematurely [1][2]. This is distinct from other timeout settings in Playwright, where specific values are used to enforce constraints on individual tests, actions, or hooks to prevent resource wastage [3][4].
Citations:
- 1: https://playwright.dev/docs/api/class-testconfig
- 2: https://github.com/microsoft/playwright/blob/c0cc9802/docs/src/test-api/class-testconfig.md
- 3: https://playwright.dev/docs/test-timeouts
- 4: https://github.com/microsoft/playwright/blob/main/docs/src/test-timeouts-js.md
Honor an explicit zero timeout override.
GLOBAL_TIMEOUT_MS=0 is treated as unset in CI because of ||, so the 110-minute default still applies. Check for an explicit env value before falling back.
🤖 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/playwright.config.ts` at line 52, Update the globalTimeout
configuration expression to distinguish an explicitly provided GLOBAL_TIMEOUT_MS
value of "0" from an unset or invalid value. Use the parsed environment value
whenever GLOBAL_TIMEOUT_MS is explicitly present, and only apply the isCI-based
110-minute default when the variable is absent.
Source: Path instructions
| }, | ||
|
|
||
| workers: process.env.WORKERS ? parseInt(process.env.WORKERS, 10) : isCI ? 1 : undefined, | ||
| workers: process.env.WORKERS ? parseInt(process.env.WORKERS, 10) : undefined, |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n 'WORKERS|workers' frontend test-prow-playwright-e2e.sh
rg -n '"`@playwright/test`"|`@playwright/test`@' package.json yarn.lock frontend/package.json frontend/yarn.lock 2>/dev/null || trueRepository: openshift/console
Length of output: 694
Preserve WORKERS syntax instead of coercing it with parseInt.
parseInt accepts malformed values like 2foo and strips supported Playwright percentage forms such as 50%. Validate the env var first, or pass through a valid worker string unchanged.
🤖 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/playwright.config.ts` at line 88, Update the workers configuration
to preserve valid WORKERS values, including Playwright percentage strings,
instead of coercing them with parseInt. Validate the environment value before
use and pass the original string through unchanged when valid, while retaining
undefined when WORKERS is unset and rejecting malformed values such as “2foo”.
Source: Path instructions
|
/pipeline required |
|
Scheduling tests matching the |
1 similar comment
|
Scheduling tests matching the |
|
/test e2e-gcp-console |
|
/test e2e-playwright Tests completed in 46m42s, but there were 5 failures of flaky tests. |
poll-console-updates: Rewrite to use mutable closure variables with
single route handlers instead of stacking multiple page.route() calls.
Wait for two check-updates responses before switching payloads so the
component's prev/current comparison refs are initialized. Add
--disable-background-timer-throttling to prevent Chromium from
throttling the 15s polling interval under CI load.
other-routes events: Replace getByRole('row').first() with
getByTestId('event-totals') to avoid asserting visibility on a
react-virtualized row that may be positioned off-screen.
yaml-editor: Increase waitForEditorReady timeout from 30s to 60s with
a two-phase wait (mounting then ready) to accommodate the multi-stage
lazy loading chain in CI.
favorites: Clear stale favorites via patchConfigMap before the test
runs and increase the no-favorites-message assertion timeout to allow
user preferences to load.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/tests/console/app/poll-console-updates.spec.ts`:
- Around line 81-88: Update the polling sequence in the test around
updatesPayload and refresh-web-console so it explicitly waits for the
error-state poll to complete before setting manifestAbort to false. Ensure the
manifest remains aborted while the updated plugin list is observed, then enable
it and verify the toast behavior, rather than relying on the immediate negative
attachment assertion.
In `@frontend/e2e/tests/console/favorites/favorites.spec.ts`:
- Around line 8-17: Update the error handling in the “Clear any stale favorites
from prior runs” test step to ignore only a 404 indicating the ConfigMap is
missing. Re-throw authentication, API, patch, and all other errors so the test
fails rather than continuing with stale favorites.
🪄 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: df6254d0-a787-4d10-8a17-5e5b0f3d4179
📒 Files selected for processing (5)
frontend/e2e/pages/yaml-editor-page.tsfrontend/e2e/tests/console/app/poll-console-updates.spec.tsfrontend/e2e/tests/console/crud/other-routes.spec.tsfrontend/e2e/tests/console/favorites/favorites.spec.tsfrontend/playwright.config.ts
| await test.step('Clear any stale favorites from prior runs', async () => { | ||
| try { | ||
| await k8sClient.patchConfigMap( | ||
| 'user-settings-kubeadmin', | ||
| 'openshift-console-user-settings', | ||
| { 'console.favorites': '[]' }, | ||
| ); | ||
| } catch { | ||
| // ConfigMap may not exist yet | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 '\bpatchConfigMap\s*\(' frontend/e2eRepository: openshift/console
Length of output: 2498
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '369,430p' frontend/e2e/clients/kubernetes-client.tsRepository: openshift/console
Length of output: 2204
Only ignore a missing ConfigMap here
Do not suppress patch failures other than a 404 from the ConfigMap lookup; auth, API, or patch errors should fail the test instead of continuing with stale favorites.
🤖 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 8 - 17,
Update the error handling in the “Clear any stale favorites from prior runs”
test step to ignore only a 404 indicating the ConfigMap is missing. Re-throw
authentication, API, patch, and all other errors so the test fails rather than
continuing with stale favorites.
…es tests poll-console-updates: Add x-mock header to route.fulfill() responses and filter on it in waitForBaseline(). If the route mock fails to intercept requests in CI, the baseline promise hangs immediately instead of silently resolving from real server responses (which would cause the toast to never appear). Reduce expect timeouts from 300s to 60s since they exceed the 120s test timeout anyway. favorites: Navigate explicitly to / after warmupSPA and assert the Overview heading is visible. In CI the user-settings lastNamespace can redirect to a different page where the favorite button is already active, causing the click to remove the favorite rather than open the dialog. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test e2e-playwright |
1 similar comment
|
/test e2e-playwright |
|
@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. |
2 participants
Analysis / Root cause:
The Playwright e2e suite was exceeding Prow's 2-hour hard timeout. With 299 tests, 55 retries, and a single worker (serial execution), the container was killed mid-run with exit code 127 (
ContainerFailed). This prevented clean JUnit output and artifact collection, masking the actual test results.Observed in PR #16741: https://storage.googleapis.com/test-platform-results/pr-logs/pull/openshift_console/16741/pull-ci-openshift-console-main-e2e-playwright/2079191239445450752/build-log.txt
Solution description:
Add
globalTimeouttoplaywright.config.ts— 110 minutes in CI (10 min headroom before Prow's 2h kill). Playwright exits cleanly, produces JUnit results, and the artifact copy trap runs. Overridable viaGLOBAL_TIMEOUT_MSenv var. Unlimited locally (Playwright default).Default to
WORKERS=2intest-prow-playwright-e2e.sh— cuts wall-clock time roughly in half. TheWORKERSenv var was already supported by the Playwright config but was never set. Overridable at the job level.Simplify
workersconfig — remove theisCI ? 1 : undefinedfallback sinceWORKERSis now always set in CI. Locally, Playwright uses its default (half the CPU cores).Screenshots / screen recording:
N/A — CI infrastructure only, no UI changes.
Test setup:
No special setup required.
Test cases:
grep -n globalTimeout frontend/playwright.config.ts— confirm it's setgrep -n WORKERS test-prow-playwright-e2e.sh— confirm it's exportede2e-playwrightCI job to verify the suite completes within the 2h budgetBrowser conformance:
N/A — no UI changes.
Additional info:
The individual test failures in the PR #16741 log (poll-console-updates, project-dashboard, etc.) are pre-existing flakes, not regressions. The container failure was purely a timeout issue from running too many tests serially.
🤖 Generated with Claude Code
Summary by CodeRabbit
globalTimeoutandworkersto be set via environment variables (with sensible fallbacks), and Chrome runs with reduced timer throttling. The Playwright CSP test runner also defaultsWORKERSto2when unset.