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
chore(ci): fix our npm caching strategy for our e2e nextjs tests #655
Conversation
⏱ Benchmark resultsComparing with 4c64157 largeDepsEsbuild: 13s⬆️ 11.71% increase vs. 4c64157
Legend
largeDepsZisi: 1m 6.6s⬆️ 12.83% increase vs. 4c64157
Legend
|
paths: ['CHANGELOG.md'] | ||
branches: [main, 'chore/fix-ci-npm-cache'] | ||
# Run on release PRs only, or changes to this same file | ||
paths: ['CHANGELOG.md', '.github/workflows/e2e-nextjs.yml'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this change before merging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it kind of makes sense to run the CI when the workflow file changes, so maybe we should leave it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If folks agree with it I would keep both these changes ☝️ similart to what we have with fossa
-https://github.com/netlify/zip-it-and-ship-it/blob/main/.github/workflows/fossa.yml#L6-L7 - this way we have the ability to always test this during dev if needed be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably change the branch name though, maybe to something like chore/ci-e2e-nextjs
? What do you think @eduardoboucas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. So you'll use that branch whenever you want to test the flow? Sounds good to me!
Co-authored-by: João Antunes <joao@netlify.com>
Next.js Integration Test (esbuild) |
Next.js Integration Test (default) |
- Summary
Similar to - netlify/cli#3277 - for our e2e tests seems like we clone a total of 3 projects. We need to make use of the
cache-dependency-path
property to point to each of the lockfiles individually (see here).- Test plan
- A picture of a cute animal (not mandatory but encouraged)