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

Add next experimental-test command #64352

Merged
merged 52 commits into from Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
852c078
wip
Ethan-Arrowood Apr 11, 2024
c7881df
share supported test runner types
Ethan-Arrowood Apr 11, 2024
34b22bc
.
Ethan-Arrowood Apr 11, 2024
2a5934c
.
Ethan-Arrowood Apr 11, 2024
e47cdb3
chore(cli): uncomment .usage()
samcx Apr 11, 2024
5e71cda
popsicle sticks and duct tape
Ethan-Arrowood Apr 11, 2024
abb1434
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 11, 2024
cd41ce2
fix
Ethan-Arrowood Apr 12, 2024
2601aa5
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 15, 2024
bd63c40
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 15, 2024
99de698
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 15, 2024
424a2ed
start on integration tests
Ethan-Arrowood Apr 15, 2024
0ebe076
test-runner validation
Ethan-Arrowood Apr 16, 2024
932ea29
beep boop
Ethan-Arrowood Apr 16, 2024
c44f927
move tests to e2e
Ethan-Arrowood Apr 16, 2024
2a3c8a3
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 16, 2024
73b3d0e
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 16, 2024
4c53c01
changes
Ethan-Arrowood Apr 17, 2024
7637259
add comment
Ethan-Arrowood Apr 17, 2024
fdfcf25
fixes
Ethan-Arrowood Apr 17, 2024
ff24c14
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 17, 2024
4e2b506
fix test runner args test
Ethan-Arrowood Apr 17, 2024
100e40e
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 18, 2024
e93a408
fix lint issues
Ethan-Arrowood Apr 18, 2024
1ad8fba
improvements
Ethan-Arrowood Apr 18, 2024
d3a7644
fix tests
Ethan-Arrowood Apr 18, 2024
a4f42ff
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 19, 2024
bfa407d
change to install instead of install-deps
Ethan-Arrowood Apr 19, 2024
ca86512
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 22, 2024
860fa64
remove unused code and add definedConfig to withNext
Ethan-Arrowood Apr 22, 2024
593d8ad
add missing `.destroy`
Ethan-Arrowood Apr 22, 2024
aaa9478
fixes fixes fixes
Ethan-Arrowood Apr 22, 2024
03394d9
modify config values
Ethan-Arrowood Apr 22, 2024
d4d04cf
change withNext to defineConfig
Ethan-Arrowood Apr 22, 2024
5dca6ca
remove empty std out assertions
Ethan-Arrowood Apr 22, 2024
f03b956
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 22, 2024
b32e9fc
revert deep-merge change
Ethan-Arrowood Apr 22, 2024
3d0b1d3
Merge branch 'next-experimental-test' of github.com:vercel/next.js in…
Ethan-Arrowood Apr 22, 2024
9249862
fixes from review
Ethan-Arrowood Apr 22, 2024
2687eb3
more fixes, resolve type issue
Ethan-Arrowood Apr 22, 2024
1276382
oh typescript
Ethan-Arrowood Apr 22, 2024
e947996
oh typescript
Ethan-Arrowood Apr 22, 2024
adcc378
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 22, 2024
6902027
fix defineConfig one more time
Ethan-Arrowood Apr 22, 2024
c8afd70
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 22, 2024
3b7a17f
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 23, 2024
310ef22
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 23, 2024
1c93145
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 23, 2024
1b44741
update config
Ethan-Arrowood Apr 23, 2024
fa4fef1
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 23, 2024
1596290
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 23, 2024
c4a36ce
Merge branch 'canary' into next-experimental-test
Ethan-Arrowood Apr 24, 2024
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
33 changes: 32 additions & 1 deletion packages/next/src/bin/next.ts
Expand Up @@ -10,6 +10,7 @@ import { bold, cyan, italic } from '../lib/picocolors'
import { formatCliHelpOutput } from '../lib/format-cli-help-output'
import { NON_STANDARD_NODE_ENV } from '../lib/constants'
import { myParseInt } from '../server/lib/utils'
import { SUPPORTED_TEST_RUNNERS_LIST } from '../cli/next-test.js'

if (
semver.lt(process.versions.node, process.env.__NEXT_REQUIRED_NODE_VERSION!)
Expand Down Expand Up @@ -346,6 +347,36 @@ program
mod.nextTelemetry(options, arg)
)
)
.usage('[options]')

program
.command('experimental-test')
.description(
`Execute \`next/experimental/testmode\` tests using a specified test runner. The test runner defaults to 'playwright' if the \`experimental.defaultTestRunner\` configuration option or the \`--test-runner\` option are not set.`
)
.argument(
'[directory]',
`A Next.js project directory to execute the test runner on. ${italic(
'If no directory is provided, the current directory will be used.'
)}`
)
.argument(
'[test-runner-args...]',
'Any additional arguments or options to pass down to the test runner `test` command.'
)
.option(
'--test-runner [test-runner]',
`Any supported test runner. Options: ${bold(
SUPPORTED_TEST_RUNNERS_LIST.join(', ')
)}. ${italic(
"If no test runner is provided, the Next.js config option `experimental.defaultTestRunner`, or 'playwright' will be used."
)}`
)
.allowUnknownOption()
.action((directory, testRunnerArgs, options) => {
return import('../cli/next-test.js').then((mod) => {
mod.nextTest(directory, testRunnerArgs, options)
})
})
.usage('[directory] [options]')

program.parse(process.argv)
195 changes: 195 additions & 0 deletions packages/next/src/cli/next-test.ts
@@ -0,0 +1,195 @@
import { writeFileSync } from 'fs'
import { getProjectDir } from '../lib/get-project-dir'
import { printAndExit } from '../server/lib/utils'
import loadConfig from '../server/config'
import { PHASE_PRODUCTION_BUILD } from '../shared/lib/constants'
import {
hasNecessaryDependencies,
type MissingDependency,
} from '../lib/has-necessary-dependencies'
import { installDependencies } from '../lib/install-dependencies'
import type { NextConfigComplete } from '../server/config-shared'
import findUp from 'next/dist/compiled/find-up'
import { findPagesDir } from '../lib/find-pages-dir'
import { verifyTypeScriptSetup } from '../lib/verify-typescript-setup'
import path from 'path'
import spawn from 'next/dist/compiled/cross-spawn'

export interface NextTestOptions {
testRunner?: string
}

export const SUPPORTED_TEST_RUNNERS_LIST = ['playwright'] as const
export type SupportedTestRunners = (typeof SUPPORTED_TEST_RUNNERS_LIST)[number]

const requiredPackagesByTestRunner: {
[k in SupportedTestRunners]: MissingDependency[]
} = {
playwright: [
{ file: 'playwright', pkg: '@playwright/test', exportsRestrict: false },
],
}

export async function nextTest(
directory?: string,
testRunnerArgs: string[] = [],
options: NextTestOptions = {}
) {
// The following mess is in order to support an existing Next.js CLI pattern of optionally, passing a project `directory` as the first argument to execute the command on.
Copy link
Member

Choose a reason for hiding this comment

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

Can we just not support this pattern instead for now until we discussed internally if we shouldn't get rid of this pattern completely? It's an experimental feature after all so I'd rather get the base functionality right without having to support each edge case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

imo its more important to support this pattern as the rest of the CLI does and then if we can make a change to it we can come through and remove it here as well. This is gross code, but it works. Same "its experimental" mentality applies I think

// This is problematic for `next test` because as a wrapper around a test runner's `test` command, it needs to pass through any additional arguments and options.
// Thus, `directory` could either be a valid Next.js project directory (that the user intends to run `next test` on), or it is the first argument for the test runner.
// Unfortunately, since many test runners support passing a path (to a test file or directory containing test files), we must check if `directory` is both a valid path and a valid Next.js project.

let baseDir, nextConfig

try {
// if directory is `undefined` or a valid path this will succeed.
baseDir = getProjectDir(directory, false)
} catch (err) {
// if that failed, then `directory` is not a valid path, so it must have meant to be the first item for `testRunnerArgs`
// @ts-expect-error directory is a string here since `getProjectDir` will succeed if its undefined
testRunnerArgs.unshift(directory)
// intentionally set baseDir to the resolved '.' path
baseDir = getProjectDir()
}

try {
// but, `baseDir` might not be a Next.js project directory, it could be a path-like argument for the test runner (i.e. `playwright test test/foo.spec.js`)
// if this succeeds, it means that `baseDir` is a Next.js project directory
nextConfig = await loadConfig(PHASE_PRODUCTION_BUILD, baseDir)
} catch (err) {
// if it doesn't, then most likely `baseDir` is not a Next.js project directory
// @ts-expect-error directory is a string here since `getProjectDir` will succeed if its undefined
testRunnerArgs.unshift(directory)
// intentionally set baseDir to the resolved '.' path
baseDir = getProjectDir()
nextConfig = await loadConfig(PHASE_PRODUCTION_BUILD, baseDir) // let this error bubble up if the `basePath` is still not a valid Next.js project
}

// set the test runner. priority is CLI option > next config > default 'playwright'
const configuredTestRunner =
options?.testRunner ?? // --test-runner='foo'
nextConfig.experimental.defaultTestRunner ?? // { experimental: { defaultTestRunner: 'foo' }}
'playwright'

if (!nextConfig.experimental.testProxy) {
return printAndExit(
`\`next experimental-test\` requires the \`experimental.testProxy: true\` configuration option.`
)
}

// execute test runner specific function
switch (configuredTestRunner) {
case 'playwright':
return runPlaywright(baseDir, nextConfig, testRunnerArgs)
default:
return printAndExit(
`Test runner ${configuredTestRunner} is not supported.`
)
}
}

async function checkRequiredDeps(
baseDir: string,
testRunner: SupportedTestRunners
) {
const deps = await hasNecessaryDependencies(
baseDir,
requiredPackagesByTestRunner[testRunner]
)
if (deps.missing.length > 0) {
await installDependencies(baseDir, deps.missing, true)

const playwright = spawn(
Copy link
Member

Choose a reason for hiding this comment

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

Nit for future, maybe swap with execa, it already does promises!

path.join(baseDir, 'node_modules', '.bin', 'playwright'),
['install'],
{
cwd: baseDir,
shell: false,
stdio: 'inherit',
env: {
...process.env,
},
}
)

return new Promise((resolve, reject) => {
playwright.on('close', (c) => resolve(c))
playwright.on('error', (err) => reject(err))
})
}
}

async function runPlaywright(
baseDir: string,
nextConfig: NextConfigComplete,
testRunnerArgs: string[]
) {
await checkRequiredDeps(baseDir, 'playwright')

const playwrightConfigFile = await findUp(
['playwright.config.js', 'playwright.config.ts'],
Copy link
Member

Choose a reason for hiding this comment

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

Does playwright also support .mjs or .json files for config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. I can only find references to these two in the docs

{
cwd: baseDir,
}
)

if (!playwrightConfigFile) {
const { pagesDir, appDir } = findPagesDir(baseDir)

const { version: typeScriptVersion } = await verifyTypeScriptSetup({
dir: baseDir,
distDir: nextConfig.distDir,
intentDirs: [pagesDir, appDir].filter(Boolean) as string[],
typeCheckPreflight: false,
tsconfigPath: nextConfig.typescript.tsconfigPath,
disableStaticImages: nextConfig.images.disableStaticImages,
hasAppDir: !!appDir,
hasPagesDir: !!pagesDir,
})

const isUsingTypeScript = !!typeScriptVersion

const playwrightConfigFilename = isUsingTypeScript
? 'playwright.config.ts'
: 'playwright.config.js'

writeFileSync(
path.join(baseDir, playwrightConfigFilename),
defaultPlaywrightConfig(isUsingTypeScript)
)

return printAndExit(
`Successfully generated ${playwrightConfigFilename}. Create your first test and then run \`next experimental-test\`.`,
0
)
} else {
const playwright = spawn(
path.join(baseDir, 'node_modules', '.bin', 'playwright'),
Copy link
Contributor

@kevva kevva Apr 25, 2024

Choose a reason for hiding this comment

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

This doesn't work in monorepos where the node_modules directory lives at top level.

['test', ...testRunnerArgs],
{
cwd: baseDir,
shell: false,
stdio: 'inherit',
env: {
...process.env,
},
}
)
return new Promise((resolve, reject) => {
playwright.on('close', (c) => resolve(c))
playwright.on('error', (err) => reject(err))
})
}
}

function defaultPlaywrightConfig(typescript: boolean) {
const comment = `/*
* Specify any additional Playwright config options here.
* They will be deep merged with Next.js' default Playwright config.
* You can access the default config by using a function: \`withNext((config) => {})\`
*/`
return typescript
Copy link
Member

Choose a reason for hiding this comment

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

Do we do the same string concetanation for default tsconfig or eslint config? Reading from a file seems more robust and means existing validation tools in CI check that the config file is actually valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mentioned this before, but our TS config fails when we make this standalone because the import path next/experimental/... doesn't make sense to it. To avoid that I have to then add the file to ignore paths and all of a sudden we aren't really validating it at all.

? `import { withNext } from 'next/experimental/testmode/playwright';\n\n${comment}\nexport default withNext();`
: `const { withNext } = require('next/experimental/testmode/playwright');\n\n${comment}\nmodule.exports = withNext();`
Ethan-Arrowood marked this conversation as resolved.
Show resolved Hide resolved
}
@@ -0,0 +1,43 @@
import { devices, type PlaywrightTestConfig } from '@playwright/test'
import type { NextOptionsConfig } from './next-options'

/**
* This is the default configuration generated by Playwright as of v1.43.0 with some modifications.
*
* - the `testMatch` property is configured to match all `*.spec.js` or `*.spec.ts` files within the `app` and `pages` directories
* - the `use` property is configured with a baseURL matching the expected dev server endpoint (http://127.0.0.1:3000)
* - the `webserver` property is configured to run `next dev`.
*/
export const defaultPlaywrightConfig: PlaywrightTestConfig<NextOptionsConfig> =
{
testMatch: '{app,pages}/**/*.spec.{t,j}s',
fullyParallel: true,
forbidOnly: process.env.CI === 'true',
retries: process.env.CI === 'true' ? 2 : 0,
reporter: [['list'], ['html', { open: 'never' }]],
use: {
baseURL: 'http://127.0.0.1:3000',
trace: 'on-first-retry',
},
projects: [
{
name: 'chromium',
use: { ...devices['Desktop Chrome'] },
},

{
name: 'firefox',
use: { ...devices['Desktop Firefox'] },
},

{
name: 'webkit',
use: { ...devices['Desktop Safari'] },
},
],
webServer: {
command: process.env.CI === 'true' ? 'next start' : 'next dev',
url: 'http://127.0.0.1:3000',
reuseExistingServer: process.env.CI !== 'true',
},
}
32 changes: 18 additions & 14 deletions packages/next/src/experimental/testmode/playwright/index.ts
@@ -1,29 +1,33 @@
// eslint-disable-next-line import/no-extraneous-dependencies
import * as base from '@playwright/test'
import type { NextFixture } from './next-fixture'
import type { NextOptions } from './next-options'
import type { NextOptions, NextOptionsConfig } from './next-options'
import type { NextWorkerFixture } from './next-worker-fixture'
import { applyNextWorkerFixture } from './next-worker-fixture'
import { applyNextFixture } from './next-fixture'
import { defaultPlaywrightConfig } from './default-config'

export { defaultPlaywrightConfig }

// eslint-disable-next-line import/no-extraneous-dependencies
export * from '@playwright/test'

export type { NextFixture, NextOptions }
export type { FetchHandlerResult } from '../proxy'

export interface NextOptionsConfig {
nextOptions?: NextOptions
// Export this second so it overrides the one from `@playwright/test`
export function defineConfig(
config: base.PlaywrightTestConfig<NextOptionsConfig>
): base.PlaywrightTestConfig<NextOptionsConfig> {
return base.defineConfig<NextOptionsConfig>({
...defaultPlaywrightConfig,
...config,
use: {
...defaultPlaywrightConfig.use,
...config.use,
},
})
}

export function defineConfig<T extends NextOptionsConfig, W>(
Ethan-Arrowood marked this conversation as resolved.
Show resolved Hide resolved
config: base.PlaywrightTestConfig<T, W>
): base.PlaywrightTestConfig<T, W>
export function defineConfig<T extends NextOptionsConfig = NextOptionsConfig>(
config: base.PlaywrightTestConfig<T>
): base.PlaywrightTestConfig<T> {
return base.defineConfig<T>(config)
}
export type { NextFixture, NextOptions }
export type { FetchHandlerResult } from '../proxy'

export const test = base.test.extend<
{ next: NextFixture; nextOptions: NextOptions },
Expand Down
@@ -1,3 +1,7 @@
export interface NextOptions {
fetchLoopback?: boolean
}

export interface NextOptionsConfig {
nextOptions?: NextOptions
}
20 changes: 9 additions & 11 deletions packages/next/src/lib/get-project-dir.ts
@@ -1,11 +1,12 @@
import path from 'path'
import { error, warn } from '../build/output/log'
import { warn } from '../build/output/log'
import { detectTypo } from './detect-typo'
import { realpathSync } from './realpath'
import { printAndExit } from '../server/lib/utils'

export function getProjectDir(dir?: string) {
export function getProjectDir(dir?: string, exitOnEnoent = true) {
const resolvedDir = path.resolve(dir || '.')
try {
const resolvedDir = path.resolve(dir || '.')
const realDir = realpathSync(resolvedDir)

if (
Expand All @@ -19,7 +20,7 @@ export function getProjectDir(dir?: string) {

return realDir
} catch (err: any) {
if (err.code === 'ENOENT') {
if (err.code === 'ENOENT' && exitOnEnoent) {
if (typeof dir === 'string') {
const detectedTypo = detectTypo(dir, [
'build',
Expand All @@ -28,22 +29,19 @@ export function getProjectDir(dir?: string) {
'lint',
'start',
'telemetry',
'experimental-test',
])

if (detectedTypo) {
error(
return printAndExit(
`"next ${dir}" does not exist. Did you mean "next ${detectedTypo}"?`
)
process.exit(1)
}
}

error(
`Invalid project directory provided, no such directory: ${path.resolve(
dir || '.'
)}`
return printAndExit(
`Invalid project directory provided, no such directory: ${resolvedDir}`
)
process.exit(1)
}
throw err
}
Expand Down