OCPBUGS-77867: Fix perspective switcher E2E test flake that causes 6-hour CI timeout#16114
Conversation
Assisted by: Claude Code
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-77867, which is invalid:
Comment 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. |
|
/jira refresh |
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-77867, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-77867, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
📝 WalkthroughWalkthroughThis pull request enhances the robustness of Developer perspective verification in OpenShift Console's integration tests. Changes include adding runtime verification steps to 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/packages/integration-tests-cypress/views/nav.ts (1)
6-7: Module-level state persists across tests—consider documenting or exporting a reset.Module-scoped variables in Cypress persist across all tests in the same spec file (and potentially across specs if the module is cached). While the flag prevents recursion, it also creates implicit coupling between tests.
If you keep this pattern, consider:
- Exporting a
resetDeveloperPerspectiveFlag()helper for specs that need explicit control- Documenting this behavior so future test authors understand the single-attempt semantics
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/integration-tests-cypress/views/nav.ts` around lines 6 - 7, The module-level flag developerPerspectiveCheckAttempted creates persistent state across tests; add a small exported helper like resetDeveloperPerspectiveFlag() and call it from specs that need a fresh state, or at minimum export and document the flag's single-attempt semantics so test authors can reset or understand behavior; locate the flag declaration (developerPerspectiveCheckAttempted) in nav.ts and implement/export the reset function and update module documentation/comments accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/integration-tests-cypress/views/nav.ts`:
- Around line 65-80: The guard flag developerPerspectiveCheckAttempted can
remain true if checkDeveloperPerspective() or the subsequent Cypress commands
fail, causing cascading test failures; wrap the call chain (the
checkDeveloperPerspective() invocation and the cy.byLegacyTestID(...).click()
chain) inside a cy.wrap().then(...) and ensure
developerPerspectiveCheckAttempted is set back to false in both the success and
failure handlers (i.e., reset the flag in the then fulfillment and in the
rejection/error handler so it always runs), or alternatively provide/reset
developerPerspectiveCheckAttempted in a beforeEach by exporting a reset helper —
update the code around developerPerspectiveCheckAttempted and the
checkDeveloperPerspective()/perspective-switcher actions to implement this.
---
Nitpick comments:
In `@frontend/packages/integration-tests-cypress/views/nav.ts`:
- Around line 6-7: The module-level flag developerPerspectiveCheckAttempted
creates persistent state across tests; add a small exported helper like
resetDeveloperPerspectiveFlag() and call it from specs that need a fresh state,
or at minimum export and document the flag's single-attempt semantics so test
authors can reset or understand behavior; locate the flag declaration
(developerPerspectiveCheckAttempted) in nav.ts and implement/export the reset
function and update module documentation/comments accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4b58444-d008-4720-9c22-d3055d912e4f
📒 Files selected for processing (3)
frontend/packages/dev-console/integration-tests/support/pages/functions/checkDeveloperPerspective.tsfrontend/packages/integration-tests-cypress/tests/crud/other-routes.cy.tsfrontend/packages/integration-tests-cypress/views/nav.ts
💤 Files with no reviewable changes (1)
- frontend/packages/integration-tests-cypress/tests/crud/other-routes.cy.ts
Revert nav.ts recursion guard and other-routes.cy.ts initAdmin removal per review feedback. The essential fix is the post-reload verification in checkDeveloperPerspective that confirms the Developer perspective is actually available after the oc patch + rollout + reload sequence. Assisted by: Claude Code
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto, sg00dwin 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 |
|
/verified by @sg00dwin |
|
@sg00dwin: 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. |
|
@sg00dwin: 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. |
|
@sg00dwin: Jira Issue Verification Checks: Jira Issue OCPBUGS-77867 Jira Issue OCPBUGS-77867 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
Fix included in accepted release 4.22.0-0.nightly-2026-03-10-100251 |
Summary
checkDeveloperPerspective()to confirm the Developer perspective is actually available (with a 30s timeout) before returning, preventing an unbounded retry loop that hangs CI for 6 hoursRoot cause
After patching
console.operator.openshift.io/clusterto enable the Developer perspective,checkDeveloperPerspective()returned without verifying the perspective was actually available. The config pipeline (oc patch→ operator reconcile → console rollout →SERVER_FLAGS.perspectivesupdate) can have delays, and when the cluster was slow to reconcile,changePerspectiveTo('Developer')would callcheckDeveloperPerspective()again, creating an infinite loop with no exit condition.This accounted for ~13% of
e2e-gcp-consolefailures over the last 14 days.Test plan
e2e-gcp-consolepasses without the "perspective switcher menu refreshed" hangAssisted by: Claude Code