-
Notifications
You must be signed in to change notification settings - Fork 91
test: next.js repo tests adjustments #3168
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
Conversation
| IS_WEBPACK_TEST: ${{ steps.decide-default-bundler.outputs.default_bundler == 'webpack' && '1' || '' }} | ||
| IS_TURBOPACK_TEST: ${{ steps.decide-default-bundler.outputs.default_bundler == 'turbopack' && '1' || '' }} |
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.
those env vars are used by next test helpers and will impact isTurbopack etc checks - for example like one used in this test https://github.com/vercel/next.js/blob/a7d3aecafedc920221a885b8ab00f35f6de5e91c/test/e2e/esm-externals/esm-externals.test.ts
📊 Package size report No changes
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
| if ( | ||
| typeof this.files === 'string' && | ||
| this.files.includes('back-forward-cache') && | ||
| process.env.NEXT_RESOLVED_VERSION && | ||
| satisfiesVersionRange(process.env.NEXT_RESOLVED_VERSION, '<=15.5.4') | ||
| ) { | ||
| require('console').log('Pinning @types/react(-dom) for back-forward-cache test fixture') | ||
| // back-forward-cache test fixture is failing types checking because: | ||
| // - @types/react(-dom) types are not pinned | ||
| // - fixture uses react `unstable_Activity` export which since fixture was introduced is no longer unstable | ||
| // and types were updated for that and no longer provide types for that export (instead provide for `Activity`) | ||
| // this adds the pinning of types to version of types that still had `unstable_Activity` type | ||
| this.dependencies['@types/react'] = '19.1.1' | ||
| this.dependencies['@types/react-dom'] = '19.1.2' | ||
| } |
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.
this is something that would fail on next.js repo as well if they run the test against current latest. The test was updated in canary, but not in latest.
I don't love this hack, but this test generally passes and I'd rather not skip it. Creating triaging issue doesn't make much sense either, so only option left is to "patch" the test when we run against current latest
| if (!this.buildCommand && this.buildArgs && this.buildArgs.length > 0) { | ||
| this.buildCommand = `next build ${this.buildArgs.join(' ')}` | ||
| } |
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.
This is to support this test https://github.com/vercel/next.js/blob/a7d3aecafedc920221a885b8ab00f35f6de5e91c/test/e2e/react-compiler/react-compiler.test.ts#L42 , without something like this --profile would not be applied and test rely on react profiling being enabled
| this.dependencies = { | ||
| ...(this.dependencies || {}), | ||
| // add the runtime package as a dependency | ||
| [runtimePackageName]: `file:${runtimePackageTarballPath}`, | ||
| } |
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.
this isn't strictly needed change, but I do find it nicer to let next.js repo e2e utils to just produce package.json that has our runtime as dependency, instead of adding it as part of installation
especially as I used this.dependencies below to "fix" a test setup for us, so all the things messing with deps is using single way of doing that
5c2900b to
ebe626f
Compare
Description
See inline comments or code comments for details of changes