OCPBUGS-105609: Fix flaky web terminal e2e tests - #16953
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@rhamilto: This pull request references Jira Issue OCPBUGS-105609, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
WalkthroughThe PR centralizes E2E credential handling, adds login during SPA warmup, replaces manual Web Terminal retries with Playwright retries, increases relevant timeouts, and marks Web Terminal tests as slow. ChangesE2E authentication flow
Web Terminal wait handling
Web Terminal test execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant WebTerminalPage
participant BasePage
participant LoginHelper
participant Console
Test->>WebTerminalPage: waitForTerminalIconVisible()
WebTerminalPage->>BasePage: warmupSPA()
BasePage->>Console: detect login page
BasePage->>LoginHelper: resolve credentials
LoginHelper-->>BasePage: developer or administrator credentials
BasePage->>Console: perform login
WebTerminalPage->>Console: reload and check terminal icon
Console-->>WebTerminalPage: terminal icon visible
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
|
@rhamilto: This pull request references Jira Issue OCPBUGS-105609, which is valid. 3 validation(s) were run on this bug
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. |
|
/pipeline required |
|
Scheduling tests matching the |
![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
🤖 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/web-terminal-config-page.ts`:
- Around line 36-38: Update clickWebTerminalTab to pass retries: 2 to
BasePage.robustClick, ensuring the Web Terminal tab click gets one retry after
the initial attempt.
🪄 Autofix
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: 31e7e82a-4828-4cfb-b273-270252ab6ad8
📒 Files selected for processing (10)
frontend/e2e/pages/base-page.tsfrontend/e2e/pages/web-terminal-config-page.tsfrontend/e2e/pages/web-terminal-page.tsfrontend/e2e/setup/admin-auth.setup.tsfrontend/e2e/setup/developer-auth.setup.tsfrontend/e2e/setup/login-helper.tsfrontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.tsfrontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.tsfrontend/e2e/tests/webterminal/web-terminal-admin.spec.tsfrontend/e2e/tests/webterminal/web-terminal-config.spec.ts
| async clickWebTerminalTab(): Promise<void> { | ||
| const tab = this.page.getByRole('tab', { name: 'Web Terminal' }); | ||
| await this.robustClick(tab, { timeout: 60_000 }); | ||
| await this.robustClick(tab, { timeout: 60_000, retries: 1 }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Set the attempt count for one retry.
Line 38 passes retries: 1. BasePage.robustClick uses this value as the total attempt count. The call therefore has no retry.
Pass retries: 2 to allow the initial click and one retry.
Proposed fix
- await this.robustClick(tab, { timeout: 60_000, retries: 1 });
+ await this.robustClick(tab, { timeout: 60_000, retries: 2 });📝 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.
| async clickWebTerminalTab(): Promise<void> { | |
| const tab = this.page.getByRole('tab', { name: 'Web Terminal' }); | |
| await this.robustClick(tab, { timeout: 60_000 }); | |
| await this.robustClick(tab, { timeout: 60_000, retries: 1 }); | |
| async clickWebTerminalTab(): Promise<void> { | |
| const tab = this.page.getByRole('tab', { name: 'Web Terminal' }); | |
| await this.robustClick(tab, { timeout: 60_000, retries: 2 }); |
🤖 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/web-terminal-config-page.ts` around lines 36 - 38, Update
clickWebTerminalTab to pass retries: 2 to BasePage.robustClick, ensuring the Web
Terminal tab click gets one retry after the initial attempt.
|
/test backend |
Addresses multiple sources of flakiness in the web terminal Playwright e2e tests: - warmupSPA now detects the login page and re-authenticates instead of retrying until timeout when the auth session has expired - waitForTerminalIconVisible replaced manual retry loop with toPass() to prevent "page closed" errors when the test timeout fires mid-reload - waitForTerminalWindow default timeout increased from 60s to 120s for slow DevWorkspace provisioning - navigateToWebTerminalConfig uses longer timeouts for the actions menu and Web Terminal tab which load dynamically - All web terminal tests marked test.slow() to triple the default timeout - Extracted getBaseURL, getAdminCredentials, getDeveloperCredentials into login-helper.ts to eliminate duplication across auth setup files and warmupSPA Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6694a93 to
f77f09d
Compare
|
/test e2e-playwright |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, 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 |
|
@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. |
|
/hold I suspect the auth issue to be fixed by #16911 is the real culprit. |
openshift-ci
Bot
added
the
do-not-merge/hold
3 participants
Analysis / Root cause:
Web terminal Playwright e2e tests are flaky in CI due to multiple independent issues:
waitForTerminalIconVisiblemanual retry loop (10×15s) exceeds the 120s test timeout afterwarmupSPAalready consumed 90s, causing "Target page closed" errorswaitForTerminalWindow60s timeout is insufficient for slow DevWorkspace provisioningSolution description:
waitForTerminalIconVisiblereplaced manual retry loop with Playwright'stoPass()pattern (120s budget, escalating intervals) which respects the test timeout and won't attempt a reload after teardownwaitForTerminalWindowdefault timeout increased from 60s to 120snavigateToWebTerminalConfiguses longer timeouts (30s for loading, 60s for actions menu) andclickWebTerminalTabusesretries: 1so the full 60s timeout goes to waiting rather than splitting across attemptstest.slow()(triples timeout from 120s to 360s)getBaseURL(),getAdminCredentials(),getDeveloperCredentials()intologin-helper.tsto eliminate credential/URL duplication across auth setup filesScreenshots / screen recording:
N/A — no visual changes
Test setup:
Cluster with Web Terminal operator installed
Test cases:
web-terminal-admin.spec.ts)web-terminal-config.spec.ts)Browser conformance:
Additional info:
https://redhat.atlassian.net/browse/OCPBUGS-105609
🤖 Generated with Claude Code