OCPBUGS-83813: Fix race conditions in debug pod Cypress tests#16305
Conversation
|
@rhamilto: This pull request references Jira Issue OCPBUGS-83813, 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 |
|
@rhamilto: This pull request references Jira Issue OCPBUGS-83813, which is valid. The bug has been moved to the POST state. 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. |
|
@rhamilto: This pull request references Jira Issue OCPBUGS-83813, 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. |
📝 WalkthroughWalkthroughThis pull request updates integration tests across multiple packages to improve test stability and readiness checks. Changes include adding document readiness verification in the admin Cypress command setup, inserting visibility assertions before clicking debug terminal links in test flows, verifying empty state UI elements are displayed, and modifying cleanup error handling to prevent test failures when resource deletion commands exit with non-zero status. 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 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/tests/app/debug-pod.cy.ts (1)
73-74: Keep visibility check and click in one Cypress chain to avoid residual flakeLine 73 and Line 74 re-query the selector between assertion and click. For this specific popover re-render bug class, chaining the commands is safer and consistent with the fix used later in the file.
Proposed change
- cy.byTestID(`popup-debug-container-link-${CONTAINER_NAME}`).should('be.visible'); - cy.byTestID(`popup-debug-container-link-${CONTAINER_NAME}`).click(); + cy.byTestID(`popup-debug-container-link-${CONTAINER_NAME}`).should('be.visible').click();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/integration-tests/tests/app/debug-pod.cy.ts` around lines 73 - 74, The test re-queries the same selector between assertion and click which can cause flakiness; replace the two separate statements that reference cy.byTestID(`popup-debug-container-link-${CONTAINER_NAME}`) with a single chained command that asserts visibility and then clicks (i.e., use .should('be.visible').click() on the same cy.byTestID(...) chain) to avoid the re-query and match the pattern used elsewhere in this spec.
🤖 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/operator-lifecycle-manager/integration-tests/tests/descriptors.cy.ts`:
- Around line 19-22: The oc delete invocation called via cy.exec should tolerate
missing resources with the native flag instead of suppressing all errors; update
the command inside the cy.exec call that uses testCRD.spec.names.kind,
testCR.metadata.name and testName to append --ignore-not-found while keeping
failOnNonZeroExit: false so NotFound is ignored but real deletion failures
(RBAC/API/server errors) still surface before checkErrors() runs.
---
Nitpick comments:
In `@frontend/packages/integration-tests/tests/app/debug-pod.cy.ts`:
- Around line 73-74: The test re-queries the same selector between assertion and
click which can cause flakiness; replace the two separate statements that
reference cy.byTestID(`popup-debug-container-link-${CONTAINER_NAME}`) with a
single chained command that asserts visibility and then clicks (i.e., use
.should('be.visible').click() on the same cy.byTestID(...) chain) to avoid the
re-query and match the pattern used elsewhere in this spec.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e8ffcf00-b088-4130-842d-5484cb22db02
📒 Files selected for processing (4)
frontend/packages/integration-tests/support/admin.tsfrontend/packages/integration-tests/tests/app/debug-pod.cy.tsfrontend/packages/integration-tests/tests/crud/customresourcedefinition.cy.tsfrontend/packages/operator-lifecycle-manager/integration-tests/tests/descriptors.cy.ts
📜 Review details
🔇 Additional comments (3)
frontend/packages/integration-tests/support/admin.ts (1)
18-18: Good stabilization gate before perspective switch.This readiness assertion is a solid flake fix and keeps
initAdmin()behavior consistent withinitDeveloper(), improving reliability for tests that chain actions right aftercy.initAdmin().frontend/packages/integration-tests/tests/crud/customresourcedefinition.cy.ts (1)
103-103: Good readiness gate for empty-state flow.Line 103 adds a deterministic visibility check before continuing, which is a solid flake-prevention improvement for this path.
frontend/packages/integration-tests/tests/app/debug-pod.cy.ts (1)
85-87: Nice fix pattern for popover race conditionThe chained
should('be.visible').click()at Line 87 is the right stabilization pattern for async popover content.
The test "Opens debug terminal page from Pods Page - Status tool tip" fails intermittently because the PatternFly Popover re-renders asynchronously after opening, causing the debug link element to detach from the DOM before Cypress can click it. This fix adds proper wait conditions to ensure the popover content is stable and visible before attempting to click the debug link. The same fix is applied to the similar test on the Pod Details page. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
d1a6fe6 to
7be2940
Compare
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
@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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto, stefanonardo 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 @rhamilto |
|
@rhamilto: 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. |
|
@rhamilto: Jira Issue Verification Checks: Jira Issue OCPBUGS-83813 Jira Issue OCPBUGS-83813 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. |
Summary
Fixes intermittent test failures in the debug pod Cypress tests caused by race conditions when clicking links inside PatternFly Popovers that re-render asynchronously.
Changes
.should('be.visible')) before clicking debug container links in popover contentTest plan
cy.wait()calls)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes