Skip to content
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

Update yarn PnP tests and disable swc file reading for PnP #33236

Merged
merged 33 commits into from Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
644a278
Update yarn PnP tests and disable swc file reading for PnP
ijjk Jan 12, 2022
1106a5a
update job
ijjk Jan 12, 2022
12d6bf4
Update test
ijjk Jan 12, 2022
669d6d7
add env variable
ijjk Jan 12, 2022
0695815
update destory
ijjk Jan 13, 2022
c8d2fcc
test one
ijjk Jan 13, 2022
646f856
bump timeout
ijjk Jan 13, 2022
991f6b0
update pnp install command
ijjk Jan 13, 2022
0fd5b6e
only run pnp test
ijjk Jan 13, 2022
ee22285
add more logs
ijjk Jan 13, 2022
719d4dc
handle exit signal
ijjk Jan 13, 2022
369d023
dont inherit stdio for install
ijjk Jan 13, 2022
1cfa2bd
update server start
ijjk Jan 13, 2022
218e396
re-add test type
ijjk Jan 13, 2022
e9c5677
add build log
ijjk Jan 13, 2022
9500633
additional logging
ijjk Jan 13, 2022
8bb963a
update build command
ijjk Jan 13, 2022
9aaa18c
remove separate timeout
ijjk Jan 13, 2022
c0a3d48
update install command
ijjk Jan 13, 2022
0fd6bd0
install separate for better time info
ijjk Jan 13, 2022
a100a1d
add cache pre-warming
ijjk Jan 13, 2022
f9de7fd
update yarn config
ijjk Jan 13, 2022
c38011f
enable other pnp tests
ijjk Jan 13, 2022
0766c3b
Separate out tests
ijjk Jan 13, 2022
cc86f89
fix-lint
ijjk Jan 13, 2022
fed6184
update path
ijjk Jan 13, 2022
3a6e924
update test concurrency for isolated tests
ijjk Jan 13, 2022
4925c87
update retries
ijjk Jan 13, 2022
0536c9c
Revert "update test concurrency for isolated tests"
ijjk Jan 13, 2022
d794a87
re-enable production tests
ijjk Jan 13, 2022
27c84ba
Merge branch 'canary' into fix/yarn-pnp-tests
ijjk Jan 13, 2022
f6d719f
Merge branch 'canary' into fix/yarn-pnp-tests
ijjk Jan 14, 2022
0e8a91e
apply suggestions
ijjk Jan 14, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 0 additions & 30 deletions .github/workflows/build_test_deploy.yml
Expand Up @@ -347,35 +347,6 @@ jobs:
- run: xvfb-run node run-tests.js test/integration/with-electron/test/index.test.js
if: ${{needs.build.outputs.docsChange != 'docs only change'}}

testYarnPnP:
runs-on: ubuntu-latest
needs: [build, build-native-dev]
env:
NODE_OPTIONS: '--unhandled-rejections=strict'
YARN_COMPRESSION_LEVEL: '0'
steps:
- name: Setup node
uses: actions/setup-node@v2
if: ${{ steps.docs-change.outputs.docsChange != 'docs only change' }}
with:
node-version: 14

- uses: actions/cache@v2
if: ${{needs.build.outputs.docsChange != 'docs only change'}}
id: restore-build
with:
path: ./*
key: ${{ github.sha }}-${{ github.run_number }}-${{ github.run_attempt }}

- uses: actions/download-artifact@v2
if: ${{needs.build.outputs.docsChange != 'docs only change'}}
with:
name: next-swc-dev-binary
path: packages/next-swc/native

- run: bash ./scripts/test-pnp.sh
if: ${{needs.build.outputs.docsChange != 'docs only change'}}

testsPass:
name: thank you, next
runs-on: ubuntu-latest
Expand All @@ -387,7 +358,6 @@ jobs:
checkPrecompiled,
testIntegration,
testUnit,
testYarnPnP,
testDev,
testProd,
]
Expand Down
2 changes: 2 additions & 0 deletions packages/next/build/webpack/loaders/next-swc-loader.js
Expand Up @@ -101,6 +101,8 @@ export function pitch() {
;(async () => {
let loaderOptions = this.getOptions() || {}
if (
// TODO: investigate swc file reading in PnP mode?
!process.versions.pnp &&
loaderOptions.fileReading &&
!EXCLUDED_PATHS.test(this.resourcePath) &&
Comment on lines +105 to 107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird - why isn't EXCLUDED_PATHS enough? 🤔 Is this.resourcePath the right parameter? Looking at the report in #31812 (comment) it shows that the reported path should match:

C:\test-nextjs\.yarn\__virtual__\next-virtual-c649e2e81d\0\cache\next-npm-12.0.8-aa55acca00-947f0295aa.zip\node_modules\next\dist\client\dev\amp-dev.js

Copy link
Member Author

@ijjk ijjk Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex doesn't seem to match the above case:

It seems to safer to avoid this slight optimization for the PnP case, although if it is a noticeable difference in performance we could update the regex.

Copy link
Contributor

@arcanis arcanis Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you need to escape the backslashes in the literal for the regex to match:

console.log(EXCLUDED_PATHS.test('C:\\test-nextjs\\.yarn\\__virtual__\\next-virtual-c649e2e81d\\0\\cache\\next-npm-12.0.8-aa55acca00-947f0295aa.zip\\node_modules\\next\\dist\\client\\dev\\amp-dev.js'))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, hmm yeah I'm not able to find any mis-matches shared in the errors from the threads 🤔

this.loaders.length - 1 === this.loaderIndex &&
Expand Down
1 change: 1 addition & 0 deletions run-tests.js
Expand Up @@ -36,6 +36,7 @@ const cleanUpAndExit = async (code) => {
if (process.env.NEXT_TEST_STARTER) {
await fs.remove(process.env.NEXT_TEST_STARTER)
}
console.log(`exiting with code ${code}`)
process.exit(code)
}

Expand Down
43 changes: 0 additions & 43 deletions scripts/test-pnp.sh

This file was deleted.

5 changes: 5 additions & 0 deletions test/e2e/yarn-pnp/test/pwa-example.test.ts
@@ -0,0 +1,5 @@
import { runTests } from './utils'

describe('yarn PnP', () => {
runTests('progressive-web-app')
})
49 changes: 49 additions & 0 deletions test/e2e/yarn-pnp/test/utils.ts
@@ -0,0 +1,49 @@
import fs from 'fs-extra'
import { join } from 'path'
import { fetchViaHTTP } from 'next-test-utils'
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'

jest.setTimeout(2 * 60 * 1000)

export function runTests(example = '') {
let next: NextInstance

beforeAll(async () => {
const srcDir = join(__dirname, '../../../../examples', example)
const srcFiles = await fs.readdir(srcDir)

const packageJson = await fs.readJson(join(srcDir, 'package.json'))

next = await createNext({
files: srcFiles.reduce((prev, file) => {
if (file !== 'package.json') {
prev[file] = new FileRef(join(srcDir, file))
}
return prev
}, {} as { [key: string]: FileRef }),
dependencies: {
...packageJson.dependencies,
...packageJson.devDependencies,
},
installCommand: ({ dependencies }) => {
const pkgs = Object.keys(dependencies).reduce((prev, cur) => {
prev.push(`${cur}@${dependencies[cur]}`)
return prev
}, [] as string[])
return `yarn set version berry && yarn config set enableGlobalCache true && yarn config set compressionLevel 0 && yarn add ${pkgs.join(
ijjk marked this conversation as resolved.
Show resolved Hide resolved
' '
)}`
},
buildCommand: `yarn next build --no-lint`,
startCommand: (global as any).isNextDev ? `yarn next` : `yarn next start`,
})
})
afterAll(() => next?.destroy())

it(`should compile and serve the index page correctly ${example}`, async () => {
const res = await fetchViaHTTP(next.url, '/')
expect(res.status).toBe(200)
expect(await res.text()).toContain('<html')
})
}
5 changes: 5 additions & 0 deletions test/e2e/yarn-pnp/test/with-eslint.test.ts
@@ -0,0 +1,5 @@
import { runTests } from './utils'

describe('yarn PnP', () => {
runTests('with-eslint')
})
5 changes: 5 additions & 0 deletions test/e2e/yarn-pnp/test/with-mdx.test.ts
@@ -0,0 +1,5 @@
import { runTests } from './utils'

describe('yarn PnP', () => {
runTests('with-mdx')
})
5 changes: 5 additions & 0 deletions test/e2e/yarn-pnp/test/with-next-sass.test.ts
@@ -0,0 +1,5 @@
import { runTests } from './utils'

describe('yarn PnP', () => {
runTests('with-next-sass')
})
5 changes: 5 additions & 0 deletions test/e2e/yarn-pnp/test/with-styled-components.test.ts
@@ -0,0 +1,5 @@
import { runTests } from './utils'

describe('yarn PnP', () => {
runTests('with-styled-components')
})
5 changes: 5 additions & 0 deletions test/e2e/yarn-pnp/test/with-typescript.test.ts
@@ -0,0 +1,5 @@
import { runTests } from './utils'

describe('yarn PnP', () => {
runTests('with-typescript')
})
43 changes: 30 additions & 13 deletions test/lib/create-next-install.js
Expand Up @@ -2,10 +2,11 @@ const os = require('os')
const path = require('path')
const execa = require('execa')
const fs = require('fs-extra')
const childProcess = require('child_process')
const { linkPackages } =
require('../../.github/actions/next-stats-action/src/prepare/repo-setup')()

async function createNextInstall(dependencies) {
async function createNextInstall(dependencies, installCommand) {
const tmpDir = await fs.realpath(process.env.NEXT_TEST_DIR || os.tmpdir())
const origRepoDir = path.join(__dirname, '../../')
const installDir = path.join(tmpDir, `next-install-${Date.now()}`)
Expand Down Expand Up @@ -45,30 +46,46 @@ async function createNextInstall(dependencies) {
}

const pkgPaths = await linkPackages(tmpRepoDir)
const combinedDependencies = {
...dependencies,
next: pkgPaths.get('next'),
}

await fs.ensureDir(installDir)
await fs.writeFile(
path.join(installDir, 'package.json'),
JSON.stringify(
{
dependencies: {
...dependencies,
next: pkgPaths.get('next'),
},
dependencies: combinedDependencies,
private: true,
},
null,
2
)
)
await execa('yarn', ['install'], {
cwd: installDir,
stdio: ['ignore', 'inherit', 'inherit'],
env: {
...process.env,
YARN_CACHE_FOLDER: path.join(installDir, '.yarn-cache'),
},
})

if (installCommand) {
const installString =
typeof installCommand === 'function'
? installCommand({ dependencies: combinedDependencies })
: installCommand

console.log('running install command', installString)

childProcess.execSync(installString, {
cwd: installDir,
stdio: ['ignore', 'inherit', 'inherit'],
})
} else {
await execa('yarn', ['install'], {
cwd: installDir,
stdio: ['ignore', 'inherit', 'inherit'],
env: {
...process.env,
YARN_CACHE_FOLDER: path.join(installDir, '.yarn-cache'),
},
})
}

await fs.remove(tmpRepoDir)
return installDir
Expand Down
25 changes: 21 additions & 4 deletions test/lib/e2e-utils.ts
@@ -1,20 +1,34 @@
import path from 'path'
import assert from 'assert'
import { NextConfig } from 'next'
import { NextInstance } from './next-modes/base'
import { InstallCommand, NextInstance } from './next-modes/base'
import { NextDevInstance } from './next-modes/next-dev'
import { NextStartInstance } from './next-modes/next-start'

const testFile = module.parent.filename
const testsFolder = path.join(__dirname, '..')

let testFile
const testFileRegex = /\.test\.(js|tsx?)/

const visitedModules = new Set()
const checkParent = (mod) => {
if (!mod?.parent || visitedModules.has(mod)) return
testFile = mod.parent.filename || ''
visitedModules.add(mod)

if (!testFileRegex.test(testFile)) {
checkParent(mod.parent)
}
}
checkParent(module)

process.env.TEST_FILE_PATH = testFile

let testMode = process.env.NEXT_TEST_MODE

if (!testFile.match(/\.test\.(js|tsx?)/)) {
if (!testFileRegex.test(testFile)) {
throw new Error(
'e2e-utils imported from non-test file (must end with .test.(js,ts,tsx)'
`e2e-utils imported from non-test file ${testFile} (must end with .test.(js,ts,tsx)`
)
}

Expand Down Expand Up @@ -97,6 +111,9 @@ export async function createNext(opts: {
}
nextConfig?: NextConfig
skipStart?: boolean
installCommand?: InstallCommand
buildCommand?: string
startCommand?: string
}): Promise<NextInstance> {
try {
if (nextInstance) {
Expand Down
36 changes: 30 additions & 6 deletions test/lib/next-modes/base.ts
Expand Up @@ -8,12 +8,18 @@ import { ChildProcess } from 'child_process'
import { createNextInstall } from '../create-next-install'

type Event = 'stdout' | 'stderr' | 'error' | 'destroy'
export type InstallCommand =
| string
| ((ctx: { dependencies: { [key: string]: string } }) => string)

export class NextInstance {
protected files: {
[filename: string]: string | FileRef
}
protected nextConfig?: NextConfig
protected installCommand?: InstallCommand
protected buildCommand?: string
protected startCommand?: string
protected dependencies?: { [name: string]: string }
protected events: { [eventName: string]: Set<any> }
public testDir: string
Expand All @@ -27,6 +33,9 @@ export class NextInstance {
files,
dependencies,
nextConfig,
installCommand,
buildCommand,
startCommand,
}: {
files: {
[filename: string]: string | FileRef
Expand All @@ -35,10 +44,16 @@ export class NextInstance {
[name: string]: string
}
nextConfig?: NextConfig
installCommand?: InstallCommand
buildCommand?: string
startCommand?: string
}) {
this.files = files
this.dependencies = dependencies
this.nextConfig = nextConfig
this.installCommand = installCommand
this.buildCommand = buildCommand
this.startCommand = startCommand
this.events = {}
this.isDestroyed = false
this.isStopping = false
Expand All @@ -59,15 +74,23 @@ export class NextInstance {
`next-test-${Date.now()}-${(Math.random() * 1000) | 0}`
)

if (process.env.NEXT_TEST_STARTER && !this.dependencies) {
if (
process.env.NEXT_TEST_STARTER &&
!this.dependencies &&
!this.installCommand
) {
await fs.copy(process.env.NEXT_TEST_STARTER, this.testDir)
} else if (!skipIsolatedNext) {
this.testDir = await createNextInstall({
react: 'latest',
'react-dom': 'latest',
...this.dependencies,
})
this.testDir = await createNextInstall(
{
react: 'latest',
'react-dom': 'latest',
...this.dependencies,
},
this.installCommand
)
}
console.log('created next.js install, writing test files')

for (const filename of Object.keys(this.files)) {
const item = this.files[filename]
Expand Down Expand Up @@ -182,6 +205,7 @@ export class NextInstance {
if (!process.env.NEXT_TEST_SKIP_CLEANUP) {
await fs.remove(this.testDir)
}
console.log(`destroyed next instance`)
}

public get url() {
Expand Down