Skip to content

Commit

Permalink
Don't swallow test failures caused by POSIX signals (#32688)
Browse files Browse the repository at this point in the history
* Don't swallow test failures caused by POSIX signals

* Update tests to not import() inside jest

* Update tests

* apply suggestion

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
ramosbugs and ijjk authored Dec 21, 2021
1 parent 0eba5b2 commit 36a6e43
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 125 deletions.
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
4 changes: 4 additions & 0 deletions test/integration/production-start-no-build/test/index.test.js
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')
})
})

0 comments on commit 36a6e43

Please sign in to comment.