Skip to content

perf(ci): cache npm dependencies in Cypress init job#60738

Open
miaulalala wants to merge 3 commits into
masterfrom
perf/noid/cypress-dependency-caching
Open

perf(ci): cache npm dependencies in Cypress init job#60738
miaulalala wants to merge 3 commits into
masterfrom
perf/noid/cypress-dependency-caching

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

@miaulalala miaulalala commented May 26, 2026

Summary

  • Enables the built-in npm cache on the setup-node step in the init job (cache: 'npm')
  • Fixes intermittent cy.find() subject no longer attached to DOM failures in files-sidebar, public-share/download, and version_naming specs

Why

npm cache: The init job runs npm ci from scratch on every run. The buildjet workspace cache (key: run_id) already shares the fully-built workspace with all 11 parallel cypress runners within a single run — that part is fine. The cache: 'npm' addition speeds up the init job itself across runs by restoring the npm cache directory, so packages are pulled locally rather than from the registry when package-lock.json hasn't changed. This is a marginal win; the architecture is already sound.

Note: a composer cache step was also added initially but has been removed — composer install --no-dev on this repo has no packages to download (all require entries are PHP extensions; require-dev is skipped with --no-dev). It is effectively a dump-autoload over files already in the repo, so there is nothing to cache.

Stale DOM fixes: getActionEntryForFile/Id in FilesUtils.ts chained .find() off a cy.get() result inside a .then() callback. After the actions button click triggers a Vue re-render, the stored element reference goes stale before .find() executes. The fix collapses the two-step get + find into a single combined CSS selector so Cypress retries the whole query atomically on re-render. The same stale-alias pattern in version_naming.cy.ts is fixed with within(). Note: getActionsForFile already had a comment describing this exact problem — getActionEntryForFile just wasn't fixed alongside it.

Test plan

  • Confirm init job logs show npm cache hits on a re-run with no package-lock.json changes
  • Confirm files/files-sidebar.cy.ts, files_sharing/public-share/download.cy.ts, and files_versions/version_naming.cy.ts pass without the stale DOM errors

🤖 Generated with Claude Code

@miaulalala miaulalala requested review from skjnldsv and susnux May 26, 2026 15:02
@miaulalala miaulalala self-assigned this May 26, 2026
@miaulalala miaulalala added 3. to review Waiting for reviews performance 🚀 tests Related to tests labels May 26, 2026
@miaulalala miaulalala added this to the Nextcloud 35 milestone May 26, 2026
Anna Larch added 2 commits May 26, 2026 17:30
Adds buildjet/cache for composer vendor/ (keyed on composer.lock) and
enables setup-node's built-in npm cache so the init job skips full
reinstalls when lock files haven't changed between runs.

Signed-off-by: Anna Larch <anna@larch.dev>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
…queries

getActionEntryForFile/Id chained .find() off a cy.get() result inside a
.then() callback. After the actions button click triggers a Vue re-render,
the stored element reference goes stale before .find() executes, causing
intermittent "subject no longer attached to DOM" failures in files-sidebar,
public-share/download, and version_naming specs.

Collapse the two-step get+find into a single combined CSS selector so
Cypress retries the whole query atomically on re-render. Same fix applied
to the alias-then-find pattern in version_naming.cy.ts.

Signed-off-by: Anna Larch <anna@larch.dev>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala force-pushed the perf/noid/cypress-dependency-caching branch from a82c877 to baf1122 Compare May 26, 2026 15:30
The server's composer.json has no installable packages under --no-dev;
require only lists PHP extensions and require-dev is skipped. composer
install is effectively a dump-autoload over files already in the repo,
so there is nothing to cache.

Signed-off-by: Anna Larch <anna@larch.dev>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala changed the title perf(ci): cache composer and npm dependencies in Cypress init job perf(ci): cache npm dependencies in Cypress init job May 26, 2026
@miaulalala miaulalala requested review from come-nc and kesselb May 26, 2026 15:52
@kesselb kesselb requested a review from Copilot May 26, 2026 19:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves Cypress CI runtime by enabling npm dependency caching in the Cypress workflow’s init job, and reduces E2E flakiness by updating Cypress selectors to avoid stale DOM element references during Vue re-renders.

Changes:

  • Enable actions/setup-node built-in npm cache (cache: 'npm') in the Cypress init job.
  • Update getActionEntryForFile/Id helpers to use a single atomic selector instead of chaining .find() after a separate cy.get().
  • Refactor a versions naming assertion to use within() instead of an aliased element + chained .find().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cypress/e2e/files/FilesUtils.ts Makes action menu entry lookup atomic to avoid “subject no longer attached” failures.
cypress/e2e/files_versions/version_naming.cy.ts Uses within() to avoid stale-element chains in version row assertions.
.github/workflows/cypress.yml Enables npm cache restore/save for faster npm ci in the init job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@miaulalala miaulalala marked this pull request as ready for review May 26, 2026 19:23
@miaulalala miaulalala requested review from a team as code owners May 26, 2026 19:23
@miaulalala miaulalala requested review from ArtificialOwl, nfebe and provokateurin and removed request for a team May 26, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews performance 🚀 tests Related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants