Skip to content

Commit 36a6e43

Browse files
ramosbugsijjk
andauthored
Don't swallow test failures caused by POSIX signals (vercel#32688)
* 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>
1 parent 0eba5b2 commit 36a6e43

8 files changed

Lines changed: 153 additions & 125 deletions

File tree

jest.config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const customJestConfig = {
99
verbose: true,
1010
rootDir: 'test',
1111
modulePaths: ['<rootDir>/lib'],
12-
transformIgnorePatterns: ['/next[/\\\\]dist/'],
12+
transformIgnorePatterns: ['/next[/\\\\]dist/', '/\\.next/'],
1313
}
1414

1515
// createJestConfig is exported in this way to ensure that next/jest can load the Next.js config which is async

packages/next/server/config.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,14 @@ export default async function loadConfig(
571571
// `import()` expects url-encoded strings, so the path must be properly
572572
// escaped and (especially on Windows) absolute paths must pe prefixed
573573
// with the `file://` protocol
574-
userConfigModule = await import(pathToFileURL(path).href)
574+
if (process.env.__NEXT_TEST_MODE === 'jest') {
575+
// dynamic import does not currently work inside of vm which
576+
// jest relies on so we fall back to require for this case
577+
// https://github.com/nodejs/node/issues/35889
578+
userConfigModule = require(path)
579+
} else {
580+
userConfigModule = await import(pathToFileURL(path).href)
581+
}
575582
} catch (err) {
576583
Log.error(
577584
`Failed to load ${configFileName}, see more info here https://nextjs.org/docs/messages/next-config-error`

run-tests.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ async function main() {
275275

276276
children.add(child)
277277

278-
child.on('exit', async (code) => {
278+
child.on('exit', async (code, signal) => {
279279
children.delete(child)
280-
if (code) {
280+
if (code !== 0 || signal !== null) {
281281
if (isFinalRun && hideOutput) {
282282
// limit out to last 64kb so that we don't
283283
// run out of log room in CI
@@ -298,7 +298,13 @@ async function main() {
298298
}
299299
trimmedOutput.forEach((chunk) => process.stdout.write(chunk))
300300
}
301-
return reject(new Error(`failed with code: ${code}`))
301+
return reject(
302+
new Error(
303+
code
304+
? `failed with code: ${code}`
305+
: `failed with signal: ${signal}`
306+
)
307+
)
302308
}
303309
await fs
304310
.remove(
@@ -348,6 +354,8 @@ async function main() {
348354
await exec(`git clean -fdx "${testDir}"`)
349355
await exec(`git checkout "${testDir}"`)
350356
} catch (err) {}
357+
} else {
358+
console.error(`${test} failed due to ${err}`)
351359
}
352360
}
353361
}

test/integration/amphtml/pages/invalid-amp.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ export default function Page() {
44
return (
55
<div>
66
<p id="invalid-amp">Invalid AMP Page</p>
7-
<img src="/images/test.png"></img>
7+
<amp-video src="/cats.mp4" layout="responsive" />
88
</div>
99
)
1010
}

test/integration/amphtml/test/index.test.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { validateAMP } from 'amp-test-utils'
44
import cheerio from 'cheerio'
5-
import { readFileSync, writeFileSync } from 'fs-extra'
5+
import { readFileSync, writeFileSync, rename } from 'fs-extra'
66
import {
77
check,
88
findPort,
@@ -31,6 +31,10 @@ describe('AMP Usage', () => {
3131
let output = ''
3232

3333
beforeAll(async () => {
34+
await rename(
35+
join(appDir, 'pages/invalid-amp.js'),
36+
join(appDir, 'pages/invalid-amp.js.bak')
37+
)
3438
const result = await nextBuild(appDir, undefined, {
3539
stdout: true,
3640
stderr: true,
@@ -46,7 +50,13 @@ describe('AMP Usage', () => {
4650
server = await startApp(app)
4751
context.appPort = appPort = server.address().port
4852
})
49-
afterAll(() => stopApp(server))
53+
afterAll(async () => {
54+
await rename(
55+
join(appDir, 'pages/invalid-amp.js.bak'),
56+
join(appDir, 'pages/invalid-amp.js')
57+
)
58+
return stopApp(server)
59+
})
5060

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

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

549559
await killApp(ampDynamic)
550560

551-
expect(inspectPayload).toContain('warn')
552561
expect(inspectPayload).toContain('error')
553562
})
554563

test/integration/production-start-no-build/test/index.test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { nextServer } from 'next-test-utils'
33
import { join } from 'path'
44
const appDir = join(__dirname, '../')
55

6+
// force require usage instead of dynamic import in jest
7+
// x-ref: https://github.com/nodejs/node/issues/35889
8+
process.env.__NEXT_TEST_MODE = 'jest'
9+
610
describe('Production Usage without production build', () => {
711
it('should show error when there is no production build', async () => {
812
await expect(async () => {

test/lib/next-test-utils.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,13 @@ export async function killApp(instance) {
378378
}
379379

380380
export async function startApp(app) {
381+
// force require usage instead of dynamic import in jest
382+
// x-ref: https://github.com/nodejs/node/issues/35889
383+
process.env.__NEXT_TEST_MODE = 'jest'
384+
385+
// TODO: tests that use this should be migrated to use
386+
// the nextStart test function instead as it tests outside
387+
// of jest's context
381388
await app.prepare()
382389
const handler = app.getRequestHandler()
383390
const server = http.createServer(handler)

test/unit/isolated/config.test.ts

Lines changed: 108 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,128 +1,121 @@
11
/* eslint-env jest */
2-
import os from 'os'
32
import { join } from 'path'
43
import loadConfig from 'next/dist/server/config'
54
import { PHASE_DEVELOPMENT_SERVER } from 'next/constants'
65

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

10-
describe('config', () => {
11-
if (os.platform() === 'linux') {
12-
it('Should get the configuration', async () => {
13-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfig)
14-
expect(config.customConfig).toBe(true)
15-
})
16-
17-
it('Should pass the phase correctly', async () => {
18-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
19-
expect(config.phase).toBe(PHASE_DEVELOPMENT_SERVER)
20-
})
21-
22-
it('Should pass the defaultConfig correctly', async () => {
23-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
24-
expect(config.defaultConfig).toBeDefined()
25-
})
26-
27-
it('Should assign object defaults deeply to user config', async () => {
28-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
29-
expect(config.distDir).toEqual('.next')
30-
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
31-
})
32-
33-
it('Should pass the customConfig correctly', async () => {
34-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
35-
customConfig: true,
36-
})
37-
expect(config.customConfig).toBe(true)
38-
})
9+
// force require usage instead of dynamic import in jest
10+
// x-ref: https://github.com/nodejs/node/issues/35889
11+
process.env.__NEXT_TEST_MODE = 'jest'
3912

40-
it('Should not pass the customConfig when it is null', async () => {
41-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, null)
42-
expect(config.webpack).toBe(null)
43-
})
44-
45-
it('Should assign object defaults deeply to customConfig', async () => {
46-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
47-
customConfig: true,
48-
onDemandEntries: { custom: true },
49-
})
50-
expect(config.customConfig).toBe(true)
51-
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
52-
})
53-
54-
it('Should allow setting objects which do not have defaults', async () => {
55-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
56-
bogusSetting: { custom: true },
57-
})
58-
expect(config.bogusSetting).toBeDefined()
59-
expect(config.bogusSetting.custom).toBe(true)
60-
})
61-
62-
it('Should override defaults for arrays from user arrays', async () => {
63-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
64-
pageExtensions: ['.bogus'],
65-
})
66-
expect(config.pageExtensions).toEqual(['.bogus'])
67-
})
68-
69-
it('Should throw when an invalid target is provided', async () => {
70-
await expect(async () => {
13+
describe('config', () => {
14+
it('Should get the configuration', async () => {
15+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfig)
16+
expect(config.customConfig).toBe(true)
17+
})
18+
19+
it('Should pass the phase correctly', async () => {
20+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
21+
expect(config.phase).toBe(PHASE_DEVELOPMENT_SERVER)
22+
})
23+
24+
it('Should pass the defaultConfig correctly', async () => {
25+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
26+
expect(config.defaultConfig).toBeDefined()
27+
})
28+
29+
it('Should assign object defaults deeply to user config', async () => {
30+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, pathToConfigFn)
31+
expect(config.distDir).toEqual('.next')
32+
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
33+
})
34+
35+
it('Should pass the customConfig correctly', async () => {
36+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
37+
customConfig: true,
38+
})
39+
expect(config.customConfig).toBe(true)
40+
})
41+
42+
it('Should not pass the customConfig when it is null', async () => {
43+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, null)
44+
expect(config.webpack).toBe(null)
45+
})
46+
47+
it('Should assign object defaults deeply to customConfig', async () => {
48+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
49+
customConfig: true,
50+
onDemandEntries: { custom: true },
51+
})
52+
expect(config.customConfig).toBe(true)
53+
expect(config.onDemandEntries.maxInactiveAge).toBeDefined()
54+
})
55+
56+
it('Should allow setting objects which do not have defaults', async () => {
57+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
58+
bogusSetting: { custom: true },
59+
})
60+
expect(config.bogusSetting).toBeDefined()
61+
expect(config.bogusSetting.custom).toBe(true)
62+
})
63+
64+
it('Should override defaults for arrays from user arrays', async () => {
65+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
66+
pageExtensions: ['.bogus'],
67+
})
68+
expect(config.pageExtensions).toEqual(['.bogus'])
69+
})
70+
71+
it('Should throw when an invalid target is provided', async () => {
72+
await expect(async () => {
73+
await loadConfig(
74+
PHASE_DEVELOPMENT_SERVER,
75+
join(__dirname, '_resolvedata', 'invalid-target')
76+
)
77+
}).rejects.toThrow(/Specified target is invalid/)
78+
})
79+
80+
it('Should pass when a valid target is provided', async () => {
81+
const config = await loadConfig(
82+
PHASE_DEVELOPMENT_SERVER,
83+
join(__dirname, '_resolvedata', 'valid-target')
84+
)
85+
expect(config.target).toBe('serverless')
86+
})
87+
88+
it('Should throw an error when next.config.js is not present', async () => {
89+
await expect(
90+
async () =>
7191
await loadConfig(
7292
PHASE_DEVELOPMENT_SERVER,
73-
join(__dirname, '_resolvedata', 'invalid-target')
93+
join(__dirname, '_resolvedata', 'typescript-config')
7494
)
75-
}).rejects.toThrow(/Specified target is invalid/)
76-
})
77-
78-
it('Should pass when a valid target is provided', async () => {
79-
const config = await loadConfig(
80-
PHASE_DEVELOPMENT_SERVER,
81-
join(__dirname, '_resolvedata', 'valid-target')
82-
)
83-
expect(config.target).toBe('serverless')
84-
})
85-
86-
it('Should throw an error when next.config.js is not present', async () => {
87-
await expect(
88-
async () =>
89-
await loadConfig(
90-
PHASE_DEVELOPMENT_SERVER,
91-
join(__dirname, '_resolvedata', 'typescript-config')
92-
)
93-
).rejects.toThrow(
94-
/Configuring Next.js via .+ is not supported. Please replace the file with 'next.config.js'/
95-
)
96-
})
97-
98-
it('Should not throw an error when two versions of next.config.js are present', async () => {
99-
const config = await loadConfig(
100-
PHASE_DEVELOPMENT_SERVER,
101-
join(__dirname, '_resolvedata', 'js-ts-config')
102-
)
103-
expect(config.__test__ext).toBe('js')
104-
})
105-
106-
it('Should ignore configs set to `undefined`', async () => {
107-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
108-
target: undefined,
109-
})
110-
expect(config.target).toBe('server')
111-
})
112-
113-
it('Should ignore configs set to `null`', async () => {
114-
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
115-
target: null,
116-
})
117-
expect(config.target).toBe('server')
118-
})
119-
} else {
120-
// TODO: remove this when segfault no longer occurs with dynamic
121-
// import inside of jest due to vm usage
122-
it('should skip on non-linux platforms', () => {
123-
console.warn(
124-
'Warning!! These tests can not currently run on non-linux systems'
125-
)
126-
})
127-
}
95+
).rejects.toThrow(
96+
/Configuring Next.js via .+ is not supported. Please replace the file with 'next.config.js'/
97+
)
98+
})
99+
100+
it('Should not throw an error when two versions of next.config.js are present', async () => {
101+
const config = await loadConfig(
102+
PHASE_DEVELOPMENT_SERVER,
103+
join(__dirname, '_resolvedata', 'js-ts-config')
104+
)
105+
expect(config.__test__ext).toBe('js')
106+
})
107+
108+
it('Should ignore configs set to `undefined`', async () => {
109+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
110+
target: undefined,
111+
})
112+
expect(config.target).toBe('server')
113+
})
114+
115+
it('Should ignore configs set to `null`', async () => {
116+
const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, null, {
117+
target: null,
118+
})
119+
expect(config.target).toBe('server')
120+
})
128121
})

0 commit comments

Comments
 (0)