From 8c07ea83d43288a6b00fe66e14a4b9bec7d6d498 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 29 Dec 2022 13:13:19 -0800 Subject: [PATCH 1/3] esm: rewrite loader hooks test Rewrite the test that validates that custom loader hooks are called from being a test that depends on internals to one that spawns a child process and checks its output to confirm expected behavior. --- test/es-module/test-esm-loader-hooks.mjs | 105 ++++-------------- .../es-module-loaders/hooks-input.mjs | 92 +++++++++++++++ 2 files changed, 116 insertions(+), 81 deletions(-) create mode 100644 test/fixtures/es-module-loaders/hooks-input.mjs diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 2704fe1a52ccce..7cd967189ec1fc 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -1,83 +1,26 @@ -// Flags: --expose-internals -import { mustCall } from '../common/index.mjs'; -import esmLoaderModule from 'internal/modules/esm/loader'; -import assert from 'assert'; - -const { ESMLoader } = esmLoaderModule; - -/** - * Verify custom hooks are called with appropriate arguments. - */ -{ - const esmLoader = new ESMLoader(); - - const originalSpecifier = 'foo/bar'; - const importAssertions = { - __proto__: null, - type: 'json', - }; - const parentURL = 'file:///entrypoint.js'; - const resolvedURL = 'file:///foo/bar.js'; - const suggestedFormat = 'test'; - - function resolve(specifier, context, defaultResolve) { - assert.strictEqual(specifier, originalSpecifier); - // Ensure `context` has all and only the properties it's supposed to - assert.deepStrictEqual(Object.keys(context), [ - 'conditions', - 'importAssertions', - 'parentURL', - ]); - assert.ok(Array.isArray(context.conditions)); - assert.deepStrictEqual(context.importAssertions, importAssertions); - assert.strictEqual(context.parentURL, parentURL); - assert.strictEqual(typeof defaultResolve, 'function'); - - return { - format: suggestedFormat, - shortCircuit: true, - url: resolvedURL, - }; - } - - function load(resolvedURL, context, defaultLoad) { - assert.strictEqual(resolvedURL, resolvedURL); - assert.ok(new URL(resolvedURL)); - // Ensure `context` has all and only the properties it's supposed to - assert.deepStrictEqual(Object.keys(context), [ - 'format', - 'importAssertions', +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', () => { + it('are called with all expected arguments', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--experimental-loader', + fixtures.fileURL('/es-module-loaders/hooks-input.mjs'), + fixtures.path('/es-modules/json-modules.mjs'), ]); - assert.strictEqual(context.format, suggestedFormat); - assert.deepStrictEqual(context.importAssertions, importAssertions); - assert.strictEqual(typeof defaultLoad, 'function'); - - // This doesn't matter (just to avoid errors) - return { - format: 'module', - shortCircuit: true, - source: '', - }; - } - - const customLoader = [ - { - exports: { - // Ensure ESMLoader actually calls the custom hooks - resolve: mustCall(resolve), - load: mustCall(load), - }, - url: import.meta.url, - }, - ]; - - esmLoader.addCustomLoaders(customLoader); - // Manually trigger hooks (since ESMLoader is not actually running) - const job = await esmLoader.getModuleJob( - originalSpecifier, - parentURL, - importAssertions, - ); - await job.modulePromise; -} + assert.strictEqual(stderr, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + + const lines = stdout.split('\n'); + assert.match(lines[0], /{"url":"file:\/\/\/.*\/json-modules\.mjs","format":"test","shortCircuit":true}/); + assert.match(lines[1], /{"source":{"type":"Buffer","data":\[.*\]},"format":"module","shortCircuit":true}/); + assert.match(lines[2], /{"url":"file:\/\/\/.*\/experimental\.json","format":"test","shortCircuit":true}/); + assert.match(lines[3], /{"source":{"type":"Buffer","data":\[.*\]},"format":"json","shortCircuit":true}/); + }); +}); diff --git a/test/fixtures/es-module-loaders/hooks-input.mjs b/test/fixtures/es-module-loaders/hooks-input.mjs new file mode 100644 index 00000000000000..a84aa73e6ffcb1 --- /dev/null +++ b/test/fixtures/es-module-loaders/hooks-input.mjs @@ -0,0 +1,92 @@ +// This is expected to be used by test-esm-loader-hooks.mjs via: +// node --loader ./test/fixtures/es-module-loaders/hooks-input.mjs ./test/fixtures/es-modules/json-modules.mjs + +import assert from 'assert'; +import { write } from 'fs'; +import { readFile } from 'fs/promises'; +import { fileURLToPath } from 'url'; + + +let resolveCalls = 0; +let loadCalls = 0; + +export async function resolve(specifier, context, next) { + resolveCalls++; + let url; + + if (resolveCalls === 1) { + url = new URL(specifier).href; + assert.match(specifier, /json-modules\.mjs$/); + assert.deepStrictEqual(context.parentURL, undefined); + assert.deepStrictEqual(context.importAssertions, { + __proto__: null, + }); + } else if (resolveCalls === 2) { + url = new URL(specifier, context.parentURL).href; + assert.match(specifier, /experimental\.json$/); + assert.match(context.parentURL, /json-modules\.mjs$/); + assert.deepStrictEqual(context.importAssertions, { + __proto__: null, + type: 'json', + }); + } + + // Ensure `context` has all and only the properties it's supposed to + assert.deepStrictEqual(Object.keys(context), [ + 'conditions', + 'importAssertions', + 'parentURL', + ]); + assert.ok(Array.isArray(context.conditions)); + assert.deepStrictEqual(typeof next, 'function'); + + const returnValue = { + url, + format: 'test', + shortCircuit: true, + } + + await new Promise(resolve => write(1, `${JSON.stringify(returnValue)}\n`, resolve)); // For the test to read + + return returnValue; +} + +export async function load(url, context, next) { + loadCalls++; + const source = await readFile(fileURLToPath(url)); + let format; + + if (loadCalls === 1) { + assert.match(url, /json-modules\.mjs$/); + assert.deepStrictEqual(context.importAssertions, { + __proto__: null, + }); + format = 'module'; + } else if (loadCalls === 2) { + assert.match(url, /experimental\.json$/); + assert.deepStrictEqual(context.importAssertions, { + __proto__: null, + type: 'json', + }); + format = 'json'; + } + + assert.ok(new URL(url)); + // Ensure `context` has all and only the properties it's supposed to + assert.deepStrictEqual(Object.keys(context), [ + 'format', + 'importAssertions', + ]); + assert.deepStrictEqual(context.format, 'test'); + assert.deepStrictEqual(typeof next, 'function'); + + const returnValue = { + source, + format, + shortCircuit: true, + }; + + await new Promise(resolve => write(1, `${JSON.stringify(returnValue)}\n`, resolve)); // For the test to read + + return returnValue; +} From 7b5e3994e6a60d221ff63cc5e76ea7d63f19ebf9 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 29 Dec 2022 17:25:43 -0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- test/fixtures/es-module-loaders/hooks-input.mjs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/fixtures/es-module-loaders/hooks-input.mjs b/test/fixtures/es-module-loaders/hooks-input.mjs index a84aa73e6ffcb1..0468e74d860d73 100644 --- a/test/fixtures/es-module-loaders/hooks-input.mjs +++ b/test/fixtures/es-module-loaders/hooks-input.mjs @@ -17,7 +17,7 @@ export async function resolve(specifier, context, next) { if (resolveCalls === 1) { url = new URL(specifier).href; assert.match(specifier, /json-modules\.mjs$/); - assert.deepStrictEqual(context.parentURL, undefined); + assert.strictEqual(context.parentURL, undefined); assert.deepStrictEqual(context.importAssertions, { __proto__: null, }); @@ -32,13 +32,13 @@ export async function resolve(specifier, context, next) { } // Ensure `context` has all and only the properties it's supposed to - assert.deepStrictEqual(Object.keys(context), [ + assert.deepStrictEqual(Reflect.ownKeys(context), [ 'conditions', 'importAssertions', 'parentURL', ]); assert.ok(Array.isArray(context.conditions)); - assert.deepStrictEqual(typeof next, 'function'); + assert.strictEqual(typeof next, 'function'); const returnValue = { url, @@ -77,8 +77,8 @@ export async function load(url, context, next) { 'format', 'importAssertions', ]); - assert.deepStrictEqual(context.format, 'test'); - assert.deepStrictEqual(typeof next, 'function'); + assert.strictEqual(context.format, 'test'); + assert.strictEqual(typeof next, 'function'); const returnValue = { source, From 0f71685134aaf71a526dc7c86ff83e0768eed55d Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 29 Dec 2022 17:40:28 -0800 Subject: [PATCH 3/3] Still on main thread, so console.log still works --- test/fixtures/es-module-loaders/hooks-input.mjs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/fixtures/es-module-loaders/hooks-input.mjs b/test/fixtures/es-module-loaders/hooks-input.mjs index 0468e74d860d73..6859cfc07d9b6a 100644 --- a/test/fixtures/es-module-loaders/hooks-input.mjs +++ b/test/fixtures/es-module-loaders/hooks-input.mjs @@ -2,7 +2,6 @@ // node --loader ./test/fixtures/es-module-loaders/hooks-input.mjs ./test/fixtures/es-modules/json-modules.mjs import assert from 'assert'; -import { write } from 'fs'; import { readFile } from 'fs/promises'; import { fileURLToPath } from 'url'; @@ -46,7 +45,7 @@ export async function resolve(specifier, context, next) { shortCircuit: true, } - await new Promise(resolve => write(1, `${JSON.stringify(returnValue)}\n`, resolve)); // For the test to read + console.log(JSON.stringify(returnValue)); // For the test to validate when it parses stdout return returnValue; } @@ -86,7 +85,7 @@ export async function load(url, context, next) { shortCircuit: true, }; - await new Promise(resolve => write(1, `${JSON.stringify(returnValue)}\n`, resolve)); // For the test to read + console.log(JSON.stringify(returnValue)); // For the test to validate when it parses stdout return returnValue; }