-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add dependency caching to pnpm #1044
Comments
The noted workaround here isn't suggested anymore, and actually the current recommendation in the README is not to cache at all for pnpm. That sucks. I want to switch to pnpm for improved CI performance, but this limitation nukes most of that benefit. |
Is there no other workaround in the meantime? CI pnpm caching just isn't possible with cypress? |
Sorry that the workaround was left in this issue! I have commented it out now. I suggest you benchmark your original configuration and then measure the timing after you convert to pnpm. I would not assume anything about the benefits of caching without trying it out first. Some actions can even be slower if GitHub provided caches are used. If you decide you must have caching for pnpm then an alternative may be to set up a workflow which does not use this action ( |
Thanks Mike. pnpm install without a cache takes about a minute. With a cache takes about half that. I'm paying for Cypress.io, and want to use parallel test support, so I don't think setting up custom Cypress workflows is practical. BTW, I was considering using yarn zero installs as an alternative to that, but Cypress also doesn't support yarn pnp properly, so that's not an option. |
FWIW, the recommendation not to cache anything is opposite to what cypress.io recommends: https://docs.cypress.io/guides/continuous-integration/introduction#Caching |
Good to have the measurements!
There is also a parallel recording example in the same repo https://github.com/cypress-io/cypress-example-kitchensink/blob/master/.github/workflows/parallel.yml which could be adapted to pnpm. If you are a paying customer of Cypress Cloud you can also use the https://www.cypress.io/support channel to express your needs for better pnpm support. I'm simply an external contributor, so my insight into Cypress' plans is limited to what is published publicly.
There is an E2E example of Cypress with Yarn Modern PnP in this repo. Cypress has problems with Component Testing together with Yarn Modern PnP and I have a couple of issues open on this subject. |
@MikeMcC399 What about recommending the use of this action with our GHA, where we aren't responsible for this: https://github.com/pnpm/action-setup I just ran across it so didn't extensively look at it. |
@jennifer-shehane that just installs pnpm and optionally runs pnpm install. AFAICT, it does zero caching. Even if it did, it would certainly still have the same issue that results in the suggestion not to use caching at all. |
@bmulholland / @jennifer-shehane
|
What would you like
When the
cypress-io/github-action
processes a project with apnpm-lock.yaml
lockfile from the pnmp package manager it should cache dependencies from the pnpm store. The location of the store can be found from the commandpnpm store path
.Why is this needed?
The
cypress-io/github-action
caches dependencies from npm and from Yarn Classic for other project types (see Installation for lockfile names). Not providing this functionality for pnpm projects is inconsistent.The benefit for users of pnpm projects with larger volumes of dependencies lies in shorter run times for workflows, providing results faster. Additionally all GitHub plans have either limits on the number of action minutes used or a cost associated with action minutes, so reducing run times is also beneficial to keep inside limits or to reduce costs. (See About Billing for GitHub Actions).
## OtherAs a workaround PR test: add npm caching to pnpm examples #1043 is proposed which manually adds caching into the workflow since it is not available in the action itself. Edit: This is now available as an example.Edit: Workaround removed.
See the README > pnpm section for current recommendations.
The text was updated successfully, but these errors were encountered: