fix(e2e): add visibility waits for new-session-btn to fix timing issues#971
fix(e2e): add visibility waits for new-session-btn to fix timing issues#971Gkrumbach07 wants to merge 1 commit intomainfrom
Conversation
The E2E tests were failing because they attempted to click the
new-session-btn before the workspace page finished loading. The page
shows a loading spinner while fetching project data, and the test's
cy.request() calls don't wait for the UI to render.
Changes:
- Add `should('be.visible')` check before clicking new-session-btn in
the before() hook (with 20s timeout)
- Add `should('be.visible')` to existing timeout checks in other tests
- Ensures the button is rendered and interactive before interaction
This fixes the "Expected to find element: [data-testid="new-session-btn"],
but never found it" error that was causing all 58 tests to be skipped.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
WalkthroughUpdated Cypress e2e test file to add explicit visibility waits and assertions before interacting with the "new session" button. Changes include adding timeout parameters and Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![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 the current code and only fix it if needed.
Inline comments:
In `@e2e/cypress/e2e/sessions.cy.ts`:
- Around line 80-82: Chained element queries are inconsistent: instead of
calling cy.get(selector, { timeout: ... }) and then re-querying the same
selector before clicking, chain .click() directly off the initial cy.get call
used around lines 80–82 in sessions.cy.ts (replace the redundant
cy.get(...).click() with the original cy.get(..., { timeout: ... }).click()).
Remove the duplicate query so the click uses the timeout-aware subject and
matches the chaining pattern used elsewhere (e.g., the patterns at lines ~184
and ~248).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21e30491-c41f-468f-a65e-287178041021
📒 Files selected for processing (1)
e2e/cypress/e2e/sessions.cy.ts
| // Wait for the workspace page to finish loading before creating a session | ||
| cy.get('[data-testid="new-session-btn"]', { timeout: 20000 }).should('be.visible') | ||
| cy.get('[data-testid="new-session-btn"]').click() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant element query; chain the click for consistency.
Line 81 queries the element with a timeout, but line 82 re-queries without one. This is inconsistent with the chained pattern used in lines 184 and 248. Chain the click directly to avoid the redundant query.
Proposed fix
// Wait for the workspace page to finish loading before creating a session
- cy.get('[data-testid="new-session-btn"]', { timeout: 20000 }).should('be.visible')
- cy.get('[data-testid="new-session-btn"]').click()
+ cy.get('[data-testid="new-session-btn"]', { timeout: 20000 }).should('be.visible').click()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/cypress/e2e/sessions.cy.ts` around lines 80 - 82, Chained element queries
are inconsistent: instead of calling cy.get(selector, { timeout: ... }) and then
re-querying the same selector before clicking, chain .click() directly off the
initial cy.get call used around lines 80–82 in sessions.cy.ts (replace the
redundant cy.get(...).click() with the original cy.get(..., { timeout: ...
}).click()). Remove the duplicate query so the click uses the timeout-aware
subject and matches the chaining pattern used elsewhere (e.g., the patterns at
lines ~184 and ~248).
|
Closing this PR as obsolete. The E2E test timing issues have already been fixed on main via commit da4285c and commit 6a487e0, which refactored session creation to use direct API calls instead of UI interactions. This is a more robust solution than adding visibility waits, as it completely bypasses the UI rendering timing issues. The failures seen on PRs #962, #964, and #965 were on an older version of main that has since been updated. |
1 participant
Summary
Fixes the E2E test failures across multiple PRs by adding proper visibility checks before clicking the
new-session-btn.Problem
The E2E tests were failing with:
This caused all 58 tests to be skipped in the before() hook, failing the E2E suite on PRs #962, #964, #965, and others.
Root Cause
The workspace page (
/projects/{slug}) shows a loading spinner while fetching project data. The test'scy.request()API calls complete successfully, but the UI is still rendering. When the test immediately tries to clicknew-session-btn, the page is still showing the loading state, so the button doesn't exist yet.Solution
Added
should('be.visible')checks before clicking the button in three places:before()hook (line 80) - with 20s timeoutThis ensures the button is fully rendered and interactive before the test attempts to click it.
Test Plan
🤖 Generated with Claude Code