Skip to content

Stabilize files sidebar Cypress specs by replacing fixed waits with state-based assertions#60314

Draft
Copilot wants to merge 3 commits into
jtr/test-cypress-e2e-FilesUtils-DOM-rerenderfrom
copilot/update-cypress-sidebar-tests
Draft

Stabilize files sidebar Cypress specs by replacing fixed waits with state-based assertions#60314
Copilot wants to merge 3 commits into
jtr/test-cypress-e2e-FilesUtils-DOM-rerenderfrom
copilot/update-cypress-sidebar-tests

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026

Summary

This updates cypress/e2e/files/files-sidebar.cy.ts to remove fixed cy.wait(600) timing dependencies and make sidebar transitions assert against deterministic UI state.
The result is less flaky behavior around sidebar switching/deletion flows where DOM re-renders can race interactions.

  • Sidebar assertion helpers

    • Added local helpers to centralize repeated checks:
      • assertSidebarShows(name)
      • assertSidebarIsClosed()
      • assertSidebarHeadingNotPresent(name)
  • State-based waits instead of sleeps

    • Removed all fixed cy.wait(600) calls.
    • Replaced with assertions on:
      • sidebar heading visibility
      • sidebar closed/not visible state
      • URL containing/not containing the active file id
      • file row removal after delete
  • Flow hardening in targeted specs

    • changes the sidebar content on other file
      • assert sidebar shows file
      • assert URL contains fileId before switching
      • switch to folder
      • assert sidebar shows folder
      • assert old file heading is absent
    • closes the sidebar on delete
      • assert sidebar open + URL contains fileId before delete
      • after delete: assert sidebar closed, row gone, URL no longer contains fileId
    • changes the fileid on delete
      • assert sidebar open + URL contains otherFileId before delete
      • after delete: assert sidebar closed, other row gone, URL no longer contains otherFileId
triggerActionForFile('file', 'details')
assertSidebarShows('file')
cy.url().should('contain', `apps/files/files/${fileId}`)

triggerActionForFile('folder', 'details')
assertSidebarShows('folder')
assertSidebarHeadingNotPresent('file')

TODO

  • Review feedback and adjust assertions if any edge-case remains in CI-only timing windows

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • download.cypress.io
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/server/server/node_modules/.bin/cypress install (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

Update the Cypress sidebar tests in cypress/e2e/files/files-sidebar.cy.ts to replace the fixed cy.wait(600) calls with state-based waits and helper assertions.

Context:

  • The current tests are flaky because they rely on fixed sleeps after opening the sidebar or before deleting items.
  • There is an existing suspected DOM re-render / stale-element issue around file action handling and sidebar updates.
  • The test file currently repeats sidebar assertions inline.

Required changes:

  1. Edit cypress/e2e/files/files-sidebar.cy.ts.
  2. Introduce small local helper functions in the test file for sidebar assertions, such as:
    • a helper that asserts the sidebar is visible and shows a given heading
    • a helper that asserts the sidebar is closed / not visible / absent
  3. Replace the fixed cy.wait(600) calls with state-based waits/assertions.
  4. In the "changes the sidebar content on other file" test:
    • assert the sidebar is open for file
    • wait on stable state such as the URL containing the current file id before switching
    • then switch to folder
    • assert the sidebar shows folder
    • assert the old file heading is no longer present
  5. In the "closes the sidebar on delete" test:
    • assert the sidebar is open for file
    • assert the URL contains the current file id before deleting
    • after delete, assert the sidebar is closed, the file row is gone, and the URL no longer contains that file id
  6. In the "changes the fileid on delete" test:
    • assert the sidebar is open for other
    • assert the URL contains otherFileId before deleting
    • after delete, assert the sidebar is closed, the row for other is gone, and the URL no longer contains that file id
  7. Keep the PR in draft mode.

Also consider whether cypress/e2e/files/FilesUtils.ts should be updated in the same PR to make triggerActionForFile() and triggerActionForFileId() more robust against DOM re-renders by re-querying the visible menu after clicking the Actions button instead of depending on a possibly stale chain through aria-controls. If you include that change, keep it minimal and directly related to reducing this test flakiness.

Validation:

  • If the agent environment supports it, run the most targeted relevant test(s) for the changed Cypress files and report the outcome in the PR.
  • If execution is not possible, still create the draft PR and note that runtime validation could not be performed in the agent environment.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: I suspect we have various places where the following types of failure is a high probability: check this class for this type of problem (and the class reported in the error too) and identify a concrete solution

Assistant: Yes — the concrete problem is a Cypress stale-element / DOM re-render race, and the most likely fix is to make the action helper atomic and to stop holding onto sidebar DOM across transitions.

What failed

The failure around:

triggerActionForFile('folder', 'details')
cy.get('[data-cy-sidebar]')
	.should('be.visible')
	.findByRole('heading', { name: 'folder' })
	.should('be.visible')

is very likely caused by the UI re-rendering while triggerActionForFile() is traversing row → actions button → menu → menu item.

The helper itself is vulnerable:

export function triggerActionForFile(filename: string, actionId: string) {
	getActionButtonForFile(filename)
		.scrollIntoView()
	getActionButtonForFile(filename)
		.click({ force: true }) // force to avoid issues with overlaying file list header
	getActionEntryForFile(filename, actionId)
		.find('button')
		.should('be.visible')
		.click()
}

Why it failed

The bad assumption is: once the row action button is found and clicked, the menu lookup remains valid through the next DOM update.

But in these sidebar tests, opening details for one item and then quickly opening details for another item is exactly the kind of flow that causes:

  • row re-render
  • menu re-mount
  • sidebar replacement/animation
  • detached subject errors or transient “not found / not visible” failures

The existing cy.wait(600) in files-sidebar.cy.ts is a strong signal the test already knows this path is race-prone.

Root cause

The root cause is not line 61 itself. It is that triggerActionForFile() performs a multi-step interaction against UI that can re-render between steps, and the sidebar tests exercise that unstable path repeatedly.

There are two hot spots:

  1. Action helper
    • getActionButtonForFile(filename) and then `getActionEntryForFile(filename, action...

This pull request was created from Copilot chat.

Copilot AI and others added 2 commits May 12, 2026 02:29
Copilot AI changed the title [WIP] Update Cypress sidebar tests to use state-based waits Stabilize files sidebar Cypress specs by replacing fixed waits with state-based assertions May 12, 2026
Copilot AI requested a review from joshtrichards May 12, 2026 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants