perf(ci): cache npm dependencies in Cypress init job#60738
Open
miaulalala wants to merge 3 commits into
Open
Conversation
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>
a82c877 to
baf1122
Compare
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>
There was a problem hiding this comment.
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-nodebuilt-in npm cache (cache: 'npm') in the Cypressinitjob. - Update
getActionEntryForFile/Idhelpers to use a single atomic selector instead of chaining.find()after a separatecy.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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
setup-nodestep in theinitjob (cache: 'npm')cy.find() subject no longer attached to DOMfailures infiles-sidebar,public-share/download, andversion_namingspecsWhy
npm cache: The
initjob runsnpm cifrom 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. Thecache: 'npm'addition speeds up theinitjob itself across runs by restoring the npm cache directory, so packages are pulled locally rather than from the registry whenpackage-lock.jsonhasn'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-devon this repo has no packages to download (allrequireentries are PHP extensions;require-devis skipped with--no-dev). It is effectively adump-autoloadover files already in the repo, so there is nothing to cache.Stale DOM fixes:
getActionEntryForFile/IdinFilesUtils.tschained.find()off acy.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-stepget+findinto a single combined CSS selector so Cypress retries the whole query atomically on re-render. The same stale-alias pattern inversion_naming.cy.tsis fixed withwithin(). Note:getActionsForFilealready had a comment describing this exact problem —getActionEntryForFilejust wasn't fixed alongside it.Test plan
initjob logs show npm cache hits on a re-run with nopackage-lock.jsonchangesfiles/files-sidebar.cy.ts,files_sharing/public-share/download.cy.ts, andfiles_versions/version_naming.cy.tspass without the stale DOM errors🤖 Generated with Claude Code