-
Notifications
You must be signed in to change notification settings - Fork 86
chore: add the Next.js e2e test suite #1782
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
Merged
Merged
Conversation
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
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo canceled.
|
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo canceled.
|
✅ Deploy Preview for netlify-plugin-nextjs-export-demo canceled.
|
✅ Deploy Preview for next-plugin-edge-middleware canceled.
|
❌ Deploy Preview for nextjs-plugin-next-11-demo failed.
|
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo canceled.
|
✅ Deploy Preview for netlify-plugin-nextjs-demo canceled.
|
✅ Deploy Preview for next-i18next-demo canceled.
|
✅ Deploy Preview for nextjs-plugin-custom-routes-demo canceled.
|
✅ Deploy Preview for next-plugin-canary canceled.
|
59f5bfe
to
1b585b3
Compare
d89001f
to
b387a42
Compare
…to mk/all-the-tests
orinokai
approved these changes
Nov 24, 2022
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.
Looks good to me. This is a such a great improvement (also a technical marvel!). Just two questions:
- I'm wondering how well this would scale so we can also test previous versions, e.g. Next 11 and 12?
- Would we want to make the Report job a required pass?
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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
Adds most of the e2e test suite from the Next.js monorepo. This is a follow-on from #1773, which added the tooling and one suite.
The approach I took was to copy across all tests, fix any failures that were problems with the tooling or errors in the tests, disable any that aren't appropriate to us (e.g. they're testing client-side stuff, or dev mode), and then move them into one of three folders. The
tests
folder has suites that fully pass with no changes.The
modified-tests
folder has tests that have been changed in some way - whether to disable individual tests or to fix errors in the tests. This is so we don't try to update them from upstream without checking this. I have disabled any failing tests and commented each with// NTL Fail
. The skipped tests can be run by setting the env varRUN_SKIPPED_TESTS
.Finally there are
disabled-tests
. These are suites that we do want to pass, but currently fail all tests or almost all tests. Each of these is usually because of a single bug or incompatibility. These are all tracked in the umbrella issue.Depending on which suites are enabled, these tests run around 30-50 site deploys. I have made changes to enable this to happen efficiently. The way I have done this is by generating a matrix of GitHub Actions jobs, with one per test file (most of which would generate one site). I use
jest-junit
to report the results, and these are uploaded as task artifacts. There is then a "Report" job that runs after the matrix, which combines these reports and generates a summary of all of the results. This can be seen in the action summary page.The way that the tests are run is by re-implementing the
NextDeployInstance
class from the Next.js monorepo. The original would deploy to Vercel. This instead deploys to Netlify, using the Netlify CLI. It usesntl deploy --build --json
, which allows it to extract the deploy preview URL, against which it runs the tests. The tests all use the same site for the deploy previews, which does not have a linked repo.The builds are performed in the GitHub Action rather than in Netlify. The workflow file has comments explaining how it distributes the sites across dozens of worker jobs. The test tooling generates the site from the local fixture in a tmp directory, with the Netlify Next runtime installed as a file dependency. It uses yarn for this as it handles these better. If you pass the env var SITE_URL then it will run the tests against this rather than deploying the site. This only makes sense when testing an individual site (i.e. running one suite) because each suite uses a different test site.
Test plan
test/e2e/*-tests
Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.