From f192b83800834c1f24af66ec2c59328cf449ed31 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sat, 18 Feb 2023 06:26:36 -0800 Subject: [PATCH] esm: misc test refactors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add test specific to the event loop - move parallel tests into es-module folder - refactor fixture to add braces for if blocks - use 'os' instead of 'fs' as placeholder - spelling PR-URL: https://github.com/nodejs/node/pull/46631 Reviewed-By: Antoine du Hamel Reviewed-By: Michaël Zasso --- .../test-esm-import-meta-resolve.mjs | 3 +- test/es-module/test-esm-initialization.mjs | 2 +- test/es-module/test-esm-loader-chaining.mjs | 6 +- test/es-module/test-esm-loader-event-loop.mjs | 12 + .../test-esm-loader-spawn-promisified.mjs | 288 ++++++++++++++++++ .../test-loaders-hidden-from-users.js | 0 .../test-loaders-unknown-builtin-module.mjs | 0 .../es-module-loaders/hooks-custom.mjs | 120 +++++--- ...aders-this-value-inside-hook-functions.mjs | 4 - 9 files changed, 377 insertions(+), 58 deletions(-) create mode 100644 test/es-module/test-esm-loader-event-loop.mjs create mode 100644 test/es-module/test-esm-loader-spawn-promisified.mjs rename test/{parallel => es-module}/test-loaders-hidden-from-users.js (100%) rename test/{parallel => es-module}/test-loaders-unknown-builtin-module.mjs (100%) delete mode 100644 test/parallel/test-loaders-this-value-inside-hook-functions.mjs diff --git a/test/es-module/test-esm-import-meta-resolve.mjs b/test/es-module/test-esm-import-meta-resolve.mjs index 1fac362172ae34..dc45610b460280 100644 --- a/test/es-module/test-esm-import-meta-resolve.mjs +++ b/test/es-module/test-esm-import-meta-resolve.mjs @@ -3,8 +3,7 @@ import { mustCall } from '../common/index.mjs'; import assert from 'assert'; const dirname = import.meta.url.slice(0, import.meta.url.lastIndexOf('/') + 1); -const fixtures = dirname.slice(0, dirname.lastIndexOf('/', dirname.length - 2) + - 1) + 'fixtures/'; +const fixtures = dirname.slice(0, dirname.lastIndexOf('/', dirname.length - 2) + 1) + 'fixtures/'; (async () => { assert.strictEqual(await import.meta.resolve('./test-esm-import-meta.mjs'), diff --git a/test/es-module/test-esm-initialization.mjs b/test/es-module/test-esm-initialization.mjs index 2bfd16135a0189..aa946a50152d40 100644 --- a/test/es-module/test-esm-initialization.mjs +++ b/test/es-module/test-esm-initialization.mjs @@ -5,7 +5,7 @@ import { execPath } from 'node:process'; import { describe, it } from 'node:test'; -describe('ESM: ensure initialisation happens only once', { concurrency: true }, () => { +describe('ESM: ensure initialization happens only once', { concurrency: true }, () => { it(async () => { const { code, stderr, stdout } = await spawnPromisified(execPath, [ '--loader', diff --git a/test/es-module/test-esm-loader-chaining.mjs b/test/es-module/test-esm-loader-chaining.mjs index a42020ef42f8dd..0f67d71ece0aa4 100644 --- a/test/es-module/test-esm-loader-chaining.mjs +++ b/test/es-module/test-esm-loader-chaining.mjs @@ -9,7 +9,7 @@ const setupArgs = [ '--input-type=module', '--eval', ]; -const commonInput = 'import fs from "node:fs"; console.log(fs)'; +const commonInput = 'import os from "node:os"; console.log(os)'; const commonArgs = [ ...setupArgs, commonInput, @@ -114,11 +114,11 @@ describe('ESM: loader chaining', { concurrency: true }, () => { ); assert.match(stdout, /^resolve arg count: 3$/m); - assert.match(stdout, /specifier: 'node:fs'/); + assert.match(stdout, /specifier: 'node:os'/); assert.match(stdout, /next: \[AsyncFunction: nextResolve\]/); assert.match(stdout, /^load arg count: 3$/m); - assert.match(stdout, /url: 'node:fs'/); + assert.match(stdout, /url: 'node:os'/); assert.match(stdout, /next: \[AsyncFunction: nextLoad\]/); }); diff --git a/test/es-module/test-esm-loader-event-loop.mjs b/test/es-module/test-esm-loader-event-loop.mjs new file mode 100644 index 00000000000000..d94e55148d85d7 --- /dev/null +++ b/test/es-module/test-esm-loader-event-loop.mjs @@ -0,0 +1,12 @@ +// Flags: --experimental-loader ./test/fixtures/es-module-loaders/hooks-custom.mjs +import { mustCall } from '../common/index.mjs'; + +const done = mustCall(); + + +// Test that the process doesn't exit because of a caught exception thrown as part of dynamic import(). +for (let i = 0; i < 10; i++) { + await import('nonexistent/file.mjs').catch(() => {}); +} + +done(); diff --git a/test/es-module/test-esm-loader-spawn-promisified.mjs b/test/es-module/test-esm-loader-spawn-promisified.mjs new file mode 100644 index 00000000000000..3a107ea64816b2 --- /dev/null +++ b/test/es-module/test-esm-loader-spawn-promisified.mjs @@ -0,0 +1,288 @@ +import { spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import assert from 'node:assert'; +import { execPath } from 'node:process'; +import { describe, it } from 'node:test'; + +describe('Loader hooks throwing errors', { concurrency: true }, () => { + it('throws on nonexistent modules', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + 'import "nonexistent/file.mjs"', + ]); + + assert.match(stderr, /ERR_MODULE_NOT_FOUND/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + + it('throws on unknown extensions', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + `import '${fixtures.fileURL('/es-modules/file.unknown')}'`, + ]); + + assert.match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + + it('throws on invalid return values', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + 'import "esmHook/badReturnVal.mjs"', + ]); + + assert.match(stderr, /ERR_INVALID_RETURN_VALUE/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + + it('throws on boolean false', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + 'import "esmHook/format.false"', + ]); + + assert.match(stderr, /ERR_INVALID_RETURN_PROPERTY_VALUE/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + it('throws on boolean true', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + 'import "esmHook/format.true"', + ]); + + assert.match(stderr, /ERR_INVALID_RETURN_PROPERTY_VALUE/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + + it('throws on invalid returned object', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + 'import "esmHook/badReturnFormatVal.mjs"', + ]); + + assert.match(stderr, /ERR_INVALID_RETURN_PROPERTY_VALUE/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + + it('throws on unsupported format', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + 'import "esmHook/unsupportedReturnFormatVal.mjs"', + ]); + + assert.match(stderr, /ERR_UNKNOWN_MODULE_FORMAT/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + + it('throws on invalid format property type', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + 'import "esmHook/badReturnSourceVal.mjs"', + ]); + + assert.match(stderr, /ERR_INVALID_RETURN_PROPERTY_VALUE/); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + }); + + it('rejects dynamic imports for all of the error cases checked above', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + `import assert from 'node:assert'; + await Promise.allSettled([ + import('nonexistent/file.mjs'), + import('${fixtures.fileURL('/es-modules/file.unknown')}'), + import('esmHook/badReturnVal.mjs'), + import('esmHook/format.false'), + import('esmHook/format.true'), + import('esmHook/badReturnFormatVal.mjs'), + import('esmHook/unsupportedReturnFormatVal.mjs'), + import('esmHook/badReturnSourceVal.mjs'), + ]).then((results) => { + assert.strictEqual(results.every((result) => result.status === 'rejected'), true); + })`, + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); +}); + +describe('Loader hooks parsing modules', { concurrency: true }, () => { + it('can parse .js files as ESM', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + `import assert from 'node:assert'; + await import('${fixtures.fileURL('/es-module-loaders/js-as-esm.js')}') + .then((parsedModule) => { + assert.strictEqual(typeof parsedModule, 'object'); + assert.strictEqual(parsedModule.namedExport, 'named-export'); + })`, + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); + + it('can define .ext files as ESM', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + `import assert from 'node:assert'; + await import('${fixtures.fileURL('/es-modules/file.ext')}') + .then((parsedModule) => { + assert.strictEqual(typeof parsedModule, 'object'); + const { default: defaultExport } = parsedModule; + assert.strictEqual(typeof defaultExport, 'function'); + assert.strictEqual(defaultExport.name, 'iAmReal'); + assert.strictEqual(defaultExport(), true); + })`, + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); + + it('can predetermine the format in the custom loader resolve hook', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + `import assert from 'node:assert'; + await import('esmHook/preknownFormat.pre') + .then((parsedModule) => { + assert.strictEqual(typeof parsedModule, 'object'); + assert.strictEqual(parsedModule.default, 'hello world'); + })`, + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); + + it('can provide source for a nonexistent file', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + `import assert from 'node:assert'; + await import('esmHook/virtual.mjs') + .then((parsedModule) => { + assert.strictEqual(typeof parsedModule, 'object'); + assert.strictEqual(typeof parsedModule.default, 'undefined'); + assert.strictEqual(parsedModule.message, 'WOOHOO!'); + })`, + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); + + it('ensures that loaders have a separate context from userland', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'), + '--input-type=module', + '--eval', + `import assert from 'node:assert'; + await import('${fixtures.fileURL('/es-modules/stateful.mjs')}') + .then(({ default: count }) => { + assert.strictEqual(count(), 1); + });`, + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); + + it('ensures that user loaders are not bound to the internal loader', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/loader-this-value-inside-hook-functions.mjs'), + '--input-type=module', + '--eval', + ';', // Actual test is inside the loader module. + ]); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + }); +}); diff --git a/test/parallel/test-loaders-hidden-from-users.js b/test/es-module/test-loaders-hidden-from-users.js similarity index 100% rename from test/parallel/test-loaders-hidden-from-users.js rename to test/es-module/test-loaders-hidden-from-users.js diff --git a/test/parallel/test-loaders-unknown-builtin-module.mjs b/test/es-module/test-loaders-unknown-builtin-module.mjs similarity index 100% rename from test/parallel/test-loaders-unknown-builtin-module.mjs rename to test/es-module/test-loaders-unknown-builtin-module.mjs diff --git a/test/fixtures/es-module-loaders/hooks-custom.mjs b/test/fixtures/es-module-loaders/hooks-custom.mjs index 65dba3535c2d95..ea2ffaf7e97070 100644 --- a/test/fixtures/es-module-loaders/hooks-custom.mjs +++ b/test/fixtures/es-module-loaders/hooks-custom.mjs @@ -6,6 +6,31 @@ import count from '../es-modules/stateful.mjs'; // used to assert node-land and user-land have different contexts count(); +export function resolve(specifier, { importAssertions }, next) { + let format = ''; + + if (specifier === 'esmHook/format.false') { + format = false; + } + if (specifier === 'esmHook/format.true') { + format = true; + } + if (specifier === 'esmHook/preknownFormat.pre') { + format = 'module'; + } + + if (specifier.startsWith('esmHook')) { + return { + format, + shortCircuit: true, + url: pathToFileURL(specifier).href, + importAssertions, + }; + } + + return next(specifier); +} + /** * @param {string} url A fully resolved file url. * @param {object} context Additional info. @@ -15,63 +40,62 @@ count(); */ export function load(url, context, next) { // Load all .js files as ESM, regardless of package scope - if (url.endsWith('.js')) return next(url, { - ...context, - format: 'module', - }); + if (url.endsWith('.js')) { + return next(url, { + ...context, + format: 'module', + }); + } - if (url.endsWith('.ext')) return next(url, { - ...context, - format: 'module', - }); + if (url.endsWith('.ext')) { + return next(url, { + ...context, + format: 'module', + }); + } - if (url.endsWith('esmHook/badReturnVal.mjs')) return 'export function returnShouldBeObject() {}'; + if (url.endsWith('esmHook/badReturnVal.mjs')) { + return 'export function returnShouldBeObject() {}'; + } - if (url.endsWith('esmHook/badReturnFormatVal.mjs')) return { - format: Array(0), - shortCircuit: true, - source: '', + if (url.endsWith('esmHook/badReturnFormatVal.mjs')) { + return { + format: Array(0), + shortCircuit: true, + source: '', + }; } - if (url.endsWith('esmHook/unsupportedReturnFormatVal.mjs')) return { - format: 'foo', // Not one of the allowable inputs: no translator named 'foo' - shortCircuit: true, - source: '', + if (url.endsWith('esmHook/unsupportedReturnFormatVal.mjs')) { + return { + format: 'foo', // Not one of the allowable inputs: no translator named 'foo' + shortCircuit: true, + source: '', + }; } - if (url.endsWith('esmHook/badReturnSourceVal.mjs')) return { - format: 'module', - shortCircuit: true, - source: Array(0), + if (url.endsWith('esmHook/badReturnSourceVal.mjs')) { + return { + format: 'module', + shortCircuit: true, + source: Array(0), + }; } - if (url.endsWith('esmHook/preknownFormat.pre')) return { - format: context.format, - shortCircuit: true, - source: `const msg = 'hello world'; export default msg;` - }; + if (url.endsWith('esmHook/preknownFormat.pre')) { + return { + format: context.format, + shortCircuit: true, + source: `const msg = 'hello world'; export default msg;` + }; + } - if (url.endsWith('esmHook/virtual.mjs')) return { - format: 'module', - shortCircuit: true, - source: `export const message = 'Woohoo!'.toUpperCase();`, - }; + if (url.endsWith('esmHook/virtual.mjs')) { + return { + format: 'module', + shortCircuit: true, + source: `export const message = 'Woohoo!'.toUpperCase();`, + }; + } return next(url); } - -export function resolve(specifier, { importAssertions }, next) { - let format = ''; - - if (specifier === 'esmHook/format.false') format = false; - if (specifier === 'esmHook/format.true') format = true; - if (specifier === 'esmHook/preknownFormat.pre') format = 'module'; - - if (specifier.startsWith('esmHook')) return { - format, - shortCircuit: true, - url: pathToFileURL(specifier).href, - importAssertions, - }; - - return next(specifier); -} diff --git a/test/parallel/test-loaders-this-value-inside-hook-functions.mjs b/test/parallel/test-loaders-this-value-inside-hook-functions.mjs deleted file mode 100644 index b74b7d9e1e4c30..00000000000000 --- a/test/parallel/test-loaders-this-value-inside-hook-functions.mjs +++ /dev/null @@ -1,4 +0,0 @@ -// Flags: --experimental-loader ./test/fixtures/es-module-loaders/loader-this-value-inside-hook-functions.mjs -import '../common/index.mjs'; - -// Actual test is inside the loader module.