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

Don't swallow test failures caused by POSIX signals #32688

Merged
merged 6 commits into from
Dec 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const customJestConfig = {
verbose: true,
rootDir: 'test',
modulePaths: ['<rootDir>/lib'],
transformIgnorePatterns: ['/next[/\\\\]dist/'],
transformIgnorePatterns: ['/next[/\\\\]dist/', '/\\.next/'],
}

// createJestConfig is exported in this way to ensure that next/jest can load the Next.js config which is async
Expand Down
9 changes: 8 additions & 1 deletion packages/next/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,14 @@ export default async function loadConfig(
// `import()` expects url-encoded strings, so the path must be properly
// escaped and (especially on Windows) absolute paths must pe prefixed
// with the `file://` protocol
userConfigModule = await import(pathToFileURL(path).href)
if (process.env.__NEXT_TEST_MODE === 'jest') {
// dynamic import does not currently work inside of vm which
// jest relies on so we fall back to require for this case
// https://github.com/nodejs/node/issues/35889
userConfigModule = require(path)
} else {
userConfigModule = await import(pathToFileURL(path).href)
}
} catch (err) {
Log.error(
`Failed to load ${configFileName}, see more info here https://nextjs.org/docs/messages/next-config-error`
Expand Down
14 changes: 11 additions & 3 deletions run-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,9 @@ async function main() {

children.add(child)

child.on('exit', async (code) => {
child.on('exit', async (code, signal) => {
children.delete(child)
if (code) {
if (code !== 0 || signal !== null) {
if (isFinalRun && hideOutput) {
// limit out to last 64kb so that we don't
// run out of log room in CI
Expand All @@ -298,7 +298,13 @@ async function main() {
}
trimmedOutput.forEach((chunk) => process.stdout.write(chunk))
}
return reject(new Error(`failed with code: ${code}`))
return reject(
new Error(
code
? `failed with code: ${code}`
: `failed with signal: ${signal}`
)
)
}
await fs
.remove(
Expand Down Expand Up @@ -348,6 +354,8 @@ async function main() {
await exec(`git clean -fdx "${testDir}"`)
await exec(`git checkout "${testDir}"`)
} catch (err) {}
} else {
console.error(`${test} failed due to ${err}`)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/amphtml/pages/invalid-amp.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export default function Page() {
return (
<div>
<p id="invalid-amp">Invalid AMP Page</p>
<img src="/images/test.png"></img>
<amp-video src="/cats.mp4" layout="responsive" />
</div>
)
}
17 changes: 13 additions & 4 deletions test/integration/amphtml/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { validateAMP } from 'amp-test-utils'
import cheerio from 'cheerio'
import { readFileSync, writeFileSync } from 'fs-extra'
import { readFileSync, writeFileSync, rename } from 'fs-extra'
import {
check,
findPort,
Expand Down Expand Up @@ -31,6 +31,10 @@ describe('AMP Usage', () => {
let output = ''

beforeAll(async () => {
await rename(
join(appDir, 'pages/invalid-amp.js'),
join(appDir, 'pages/invalid-amp.js.bak')
)
const result = await nextBuild(appDir, undefined, {
stdout: true,
stderr: true,
Expand All @@ -46,7 +50,13 @@ describe('AMP Usage', () => {
server = await startApp(app)
context.appPort = appPort = server.address().port
})
afterAll(() => stopApp(server))
afterAll(async () => {
await rename(
join(appDir, 'pages/invalid-amp.js.bak'),
join(appDir, 'pages/invalid-amp.js')
)
return stopApp(server)
})

it('should have amp optimizer in trace', async () => {
const trace = JSON.parse(
Expand Down Expand Up @@ -242,7 +252,7 @@ describe('AMP Usage', () => {
const html = await renderViaHTTP(appPort, '/styled?amp=1')
const $ = cheerio.load(html)
expect($('style[amp-custom]').first().text()).toMatch(
/div.jsx-\d+{color:red}span.jsx-\d+{color:blue}body{background-color:green}/
/div.jsx-[a-zA-Z0-9]{1,}{color:red}span.jsx-[a-zA-Z0-9]{1,}{color:blue}body{background-color:green}/
)
})

Expand Down Expand Up @@ -548,7 +558,6 @@ describe('AMP Usage', () => {

await killApp(ampDynamic)

expect(inspectPayload).toContain('warn')
expect(inspectPayload).toContain('error')
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { nextServer } from 'next-test-utils'
import { join } from 'path'
const appDir = join(__dirname, '../')

// force require usage instead of dynamic import in jest
// x-ref: https://github.com/nodejs/node/issues/35889
process.env.__NEXT_TEST_MODE = 'jest'

describe('Production Usage without production build', () => {
it('should show error when there is no production build', async () => {
await expect(async () => {
Expand Down
7 changes: 7 additions & 0 deletions test/lib/next-test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,13 @@ export async function killApp(instance) {
}

export async function startApp(app) {
// force require usage instead of dynamic import in jest
// x-ref: https://github.com/nodejs/node/issues/35889
process.env.__NEXT_TEST_MODE = 'jest'

// TODO: tests that use this should be migrated to use
// the nextStart test function instead as it tests outside
// of jest's context
await app.prepare()
const handler = app.getRequestHandler()
const server = http.createServer(handler)
Expand Down
223 changes: 108 additions & 115 deletions test/unit/isolated/config.test.ts
Original file line number Diff line number Diff line change
@@ -1,128 +1,121 @@
/* eslint-env jest */
import os from 'os'
import { join } from 'path'
import loadConfig from 'next/dist/server/config'
import { PHASE_DEVELOPMENT_SERVER } from 'next/constants'

const pathToConfig = join(__dirname, '_resolvedata', 'without-function')
const pathToConfigFn = join(__dirname, '_resolvedata', 'with-function')

describe('config', () => {
if (os.platform() === 'linux') {
it('Should get the configuration', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfig)
expect(config.customConfig).toBe(true)
})

it('Should pass the phase correctly', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
expect(config.phase).toBe(PHASE_DEVELOPMENT_SERVER)
})

it('Should pass the defaultConfig correctly', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
expect(config.defaultConfig).toBeDefined()
})

it('Should assign object defaults deeply to user config', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
expect(config.distDir).toEqual('.next')
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
})

it('Should pass the customConfig correctly', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
customConfig: true,
})
expect(config.customConfig).toBe(true)
})
// force require usage instead of dynamic import in jest
// x-ref: https://github.com/nodejs/node/issues/35889
process.env.__NEXT_TEST_MODE = 'jest'

it('Should not pass the customConfig when it is null', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, null)
expect(config.webpack).toBe(null)
})

it('Should assign object defaults deeply to customConfig', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
customConfig: true,
onDemandEntries: { custom: true },
})
expect(config.customConfig).toBe(true)
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
})

it('Should allow setting objects which do not have defaults', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
bogusSetting: { custom: true },
})
expect(config.bogusSetting).toBeDefined()
expect(config.bogusSetting.custom).toBe(true)
})

it('Should override defaults for arrays from user arrays', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
pageExtensions: ['.bogus'],
})
expect(config.pageExtensions).toEqual(['.bogus'])
})

it('Should throw when an invalid target is provided', async () => {
await expect(async () => {
describe('config', () => {
it('Should get the configuration', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfig)
expect(config.customConfig).toBe(true)
})

it('Should pass the phase correctly', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
expect(config.phase).toBe(PHASE_DEVELOPMENT_SERVER)
})

it('Should pass the defaultConfig correctly', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
expect(config.defaultConfig).toBeDefined()
})

it('Should assign object defaults deeply to user config', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
expect(config.distDir).toEqual('.next')
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
})

it('Should pass the customConfig correctly', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
customConfig: true,
})
expect(config.customConfig).toBe(true)
})

it('Should not pass the customConfig when it is null', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, null)
expect(config.webpack).toBe(null)
})

it('Should assign object defaults deeply to customConfig', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
customConfig: true,
onDemandEntries: { custom: true },
})
expect(config.customConfig).toBe(true)
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
})

it('Should allow setting objects which do not have defaults', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
bogusSetting: { custom: true },
})
expect(config.bogusSetting).toBeDefined()
expect(config.bogusSetting.custom).toBe(true)
})

it('Should override defaults for arrays from user arrays', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
pageExtensions: ['.bogus'],
})
expect(config.pageExtensions).toEqual(['.bogus'])
})

it('Should throw when an invalid target is provided', async () => {
await expect(async () => {
await loadConfig(
PHASE_DEVELOPMENT_SERVER,
join(__dirname, '_resolvedata', 'invalid-target')
)
}).rejects.toThrow(/Specified target is invalid/)
})

it('Should pass when a valid target is provided', async () => {
const config = await loadConfig(
PHASE_DEVELOPMENT_SERVER,
join(__dirname, '_resolvedata', 'valid-target')
)
expect(config.target).toBe('serverless')
})

it('Should throw an error when next.config.js is not present', async () => {
await expect(
async () =>
await loadConfig(
PHASE_DEVELOPMENT_SERVER,
join(__dirname, '_resolvedata', 'invalid-target')
join(__dirname, '_resolvedata', 'typescript-config')
)
}).rejects.toThrow(/Specified target is invalid/)
})

it('Should pass when a valid target is provided', async () => {
const config = await loadConfig(
PHASE_DEVELOPMENT_SERVER,
join(__dirname, '_resolvedata', 'valid-target')
)
expect(config.target).toBe('serverless')
})

it('Should throw an error when next.config.js is not present', async () => {
await expect(
async () =>
await loadConfig(
PHASE_DEVELOPMENT_SERVER,
join(__dirname, '_resolvedata', 'typescript-config')
)
).rejects.toThrow(
/Configuring Next.js via .+ is not supported. Please replace the file with 'next.config.js'/
)
})

it('Should not throw an error when two versions of next.config.js are present', async () => {
const config = await loadConfig(
PHASE_DEVELOPMENT_SERVER,
join(__dirname, '_resolvedata', 'js-ts-config')
)
expect(config.__test__ext).toBe('js')
})

it('Should ignore configs set to `undefined`', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
target: undefined,
})
expect(config.target).toBe('server')
})

it('Should ignore configs set to `null`', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
target: null,
})
expect(config.target).toBe('server')
})
} else {
// TODO: remove this when segfault no longer occurs with dynamic
// import inside of jest due to vm usage
it('should skip on non-linux platforms', () => {
console.warn(
'Warning!! These tests can not currently run on non-linux systems'
)
})
}
).rejects.toThrow(
/Configuring Next.js via .+ is not supported. Please replace the file with 'next.config.js'/
)
})

it('Should not throw an error when two versions of next.config.js are present', async () => {
const config = await loadConfig(
PHASE_DEVELOPMENT_SERVER,
join(__dirname, '_resolvedata', 'js-ts-config')
)
expect(config.__test__ext).toBe('js')
})

it('Should ignore configs set to `undefined`', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
target: undefined,
})
expect(config.target).toBe('server')
})

it('Should ignore configs set to `null`', async () => {
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
target: null,
})
expect(config.target).toBe('server')
})
})