From 48502c2b7a65f625d870d368fded7e0eb768277b Mon Sep 17 00:00:00 2001 From: "David A. Ramos" Date: Mon, 20 Dec 2021 17:59:56 -0800 Subject: [PATCH 1/4] Don't swallow test failures caused by POSIX signals --- run-tests.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/run-tests.js b/run-tests.js index c73651fe8781b..ed1a316cd5654 100644 --- a/run-tests.js +++ b/run-tests.js @@ -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 || signal !== null) { if (isFinalRun && hideOutput) { // limit out to last 64kb so that we don't // run out of log room in CI @@ -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( @@ -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}`) } } } From 7b3612a0fe6db8b322546c0be9d28e34334fbaee Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 21 Dec 2021 10:42:54 -0600 Subject: [PATCH 2/4] Update tests to not import() inside jest --- packages/next/server/config.ts | 9 +- test/integration/amphtml/pages/invalid-amp.js | 2 +- test/integration/amphtml/test/index.test.js | 17 +- test/lib/next-test-utils.js | 7 + test/unit/isolated/config.test.ts | 223 +++++++++--------- 5 files changed, 137 insertions(+), 121 deletions(-) diff --git a/packages/next/server/config.ts b/packages/next/server/config.ts index 0df770e1418d0..01f6b902027f4 100644 --- a/packages/next/server/config.ts +++ b/packages/next/server/config.ts @@ -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` diff --git a/test/integration/amphtml/pages/invalid-amp.js b/test/integration/amphtml/pages/invalid-amp.js index 647d6ab7c05a6..e87c488a15ab8 100644 --- a/test/integration/amphtml/pages/invalid-amp.js +++ b/test/integration/amphtml/pages/invalid-amp.js @@ -4,7 +4,7 @@ export default function Page() { return (

Invalid AMP Page

- +
) } diff --git a/test/integration/amphtml/test/index.test.js b/test/integration/amphtml/test/index.test.js index e0441c76f5463..bd90039b7c458 100644 --- a/test/integration/amphtml/test/index.test.js +++ b/test/integration/amphtml/test/index.test.js @@ -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, @@ -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, @@ -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( @@ -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}/ ) }) @@ -548,7 +558,6 @@ describe('AMP Usage', () => { await killApp(ampDynamic) - expect(inspectPayload).toContain('warn') expect(inspectPayload).toContain('error') }) diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js index fd10c90a97858..6f6c6db11d27d 100644 --- a/test/lib/next-test-utils.js +++ b/test/lib/next-test-utils.js @@ -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) diff --git a/test/unit/isolated/config.test.ts b/test/unit/isolated/config.test.ts index a5f119fcb6c97..01bc0c2671fb5 100644 --- a/test/unit/isolated/config.test.ts +++ b/test/unit/isolated/config.test.ts @@ -1,5 +1,4 @@ /* 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' @@ -7,122 +6,116 @@ 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') + }) }) From 459df00d4b569bbd99501ea57455884692a4fc34 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 21 Dec 2021 11:20:35 -0600 Subject: [PATCH 3/4] Update tests --- jest.config.js | 2 +- test/integration/production-start-no-build/test/index.test.js | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index fc8f761e22838..3c9fa1cbb7132 100644 --- a/jest.config.js +++ b/jest.config.js @@ -9,7 +9,7 @@ const customJestConfig = { verbose: true, rootDir: 'test', modulePaths: ['/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 diff --git a/test/integration/production-start-no-build/test/index.test.js b/test/integration/production-start-no-build/test/index.test.js index 6f07713de8059..54896469a3aac 100644 --- a/test/integration/production-start-no-build/test/index.test.js +++ b/test/integration/production-start-no-build/test/index.test.js @@ -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 () => { From cf4b69708df7cb7c56e4a2fcacfbae843130c95a Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 21 Dec 2021 11:56:46 -0600 Subject: [PATCH 4/4] apply suggestion --- run-tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-tests.js b/run-tests.js index ed1a316cd5654..ea6a69be59de2 100644 --- a/run-tests.js +++ b/run-tests.js @@ -277,7 +277,7 @@ async function main() { child.on('exit', async (code, signal) => { children.delete(child) - if (code || signal !== null) { + 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