GeezerVison is showing github.com — test: refactor ESM tests to improve performance · nodejs/node@cbc1189
File treeExpand file treeCollapse file tree Open diff view settings fixtures/es-module-loaders
Some content is hiddenLarge Commits have some content hidden by default. Use the searchbox below for content that may be hidden. Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -336,7 +336,7 @@ class ESMLoader { | 336 | 336 | * A list of exports from user-defined loaders (as returned by | 337 | 337 | * ESMLoader.import()). | 338 | 338 | */ | 339 | | - async addCustomLoaders( | | 339 | + addCustomLoaders( | 340 | 340 | customLoaders = [], | 341 | 341 | ) { | 342 | 342 | for (let i = 0; i < customLoaders.length; i++) { | |
Collapse file Copy file name to clipboardExpand all lines: test/common/index.js +32-1Lines changed: 32 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
| 24 | 24 | const process = global.process; // Some tests tamper with the process global. | 25 | 25 |
| 26 | 26 | const assert = require('assert'); | 27 | | -const { exec, execSync, spawnSync } = require('child_process'); | | 27 | +const { exec, execSync, spawn, spawnSync } = require('child_process'); | 28 | 28 | const fs = require('fs'); | 29 | 29 | // Do not require 'os' until needed so that test-os-checked-function can | 30 | 30 | // monkey patch it. If 'os' is required here, that test will fail. | @@ -842,6 +842,36 @@ function requireNoPackageJSONAbove(dir = __dirname) { | 842 | 842 | } | 843 | 843 | } | 844 | 844 |
| | 845 | +function spawnPromisified(...args) { | | 846 | + let stderr = ''; | | 847 | + let stdout = ''; | | 848 | + | | 849 | + const child = spawn(...args); | | 850 | + child.stderr.setEncoding('utf8'); | | 851 | + child.stderr.on('data', (data) => { stderr += data; }); | | 852 | + child.stdout.setEncoding('utf8'); | | 853 | + child.stdout.on('data', (data) => { stdout += data; }); | | 854 | + | | 855 | + return new Promise((resolve, reject) => { | | 856 | + child.on('close', (code, signal) => { | | 857 | + resolve({ | | 858 | + code, | | 859 | + signal, | | 860 | + stderr, | | 861 | + stdout, | | 862 | + }); | | 863 | + }); | | 864 | + child.on('error', (code, signal) => { | | 865 | + reject({ | | 866 | + code, | | 867 | + signal, | | 868 | + stderr, | | 869 | + stdout, | | 870 | + }); | | 871 | + }); | | 872 | + }); | | 873 | +} | | 874 | + | 845 | 875 | const common = { | 846 | 876 | allowGlobals, | 847 | 877 | buildType, | @@ -891,6 +921,7 @@ const common = { | 891 | 921 | skipIfEslintMissing, | 892 | 922 | skipIfInspectorDisabled, | 893 | 923 | skipIfWorker, | | 924 | + spawnPromisified, | 894 | 925 |
| 895 | 926 | get enoughTestMem() { | 896 | 927 | return require('os').totalmem() > 0x70000000; /* 1.75 Gb */ | |
Collapse file Copy file name to clipboardExpand all lines: test/common/index.mjs +6-2Lines changed: 6 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -23,6 +23,7 @@ const { | 23 | 23 | hasCrypto, | 24 | 24 | hasIPv6, | 25 | 25 | childShouldThrowAndAbort, | | 26 | + checkoutEOL, | 26 | 27 | createZeroFilledFile, | 27 | 28 | platformTimeout, | 28 | 29 | allowGlobals, | @@ -47,7 +48,8 @@ const { | 47 | 48 | getArrayBufferViews, | 48 | 49 | getBufferSources, | 49 | 50 | getTTYfd, | 50 | | - runWithInvalidFD | | 51 | + runWithInvalidFD, | | 52 | + spawnPromisified, | 51 | 53 | } = common; | 52 | 54 |
| 53 | 55 | export { | @@ -70,6 +72,7 @@ export { | 70 | 72 | hasCrypto, | 71 | 73 | hasIPv6, | 72 | 74 | childShouldThrowAndAbort, | | 75 | + checkoutEOL, | 73 | 76 | createZeroFilledFile, | 74 | 77 | platformTimeout, | 75 | 78 | allowGlobals, | @@ -95,5 +98,6 @@ export { | 95 | 98 | getBufferSources, | 96 | 99 | getTTYfd, | 97 | 100 | runWithInvalidFD, | 98 | | - createRequire | | 101 | + createRequire, | | 102 | + spawnPromisified, | 99 | 103 | }; |
Collapse file Copy file name to clipboard +53-49Lines changed: 53 additions & 49 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | 'use strict'; | 2 | 2 |
| 3 | | -const common = require('../common'); | 4 | | -const fixtures = require('../common/fixtures'); | 5 | | -const { spawn } = require('child_process'); | 6 | | -const assert = require('assert'); | 7 | | -const path = require('path'); | | 3 | +const { spawnPromisified } = require('../common'); | | 4 | +const fixtures = require('../common/fixtures.js'); | | 5 | +const assert = require('node:assert'); | | 6 | +const path = require('node:path'); | | 7 | +const { execPath } = require('node:process'); | | 8 | +const { describe, it } = require('node:test'); | | 9 | + | 8 | 10 |
| 9 | 11 | const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js')); | 10 | 12 | const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js')); | 11 | 13 | const pjson = path.resolve( | 12 | 14 | fixtures.path('/es-modules/package-type-module/package.json') | 13 | 15 | ); | 14 | 16 |
| 15 | | -{ | 16 | | - const required = path.resolve( | 17 | | - fixtures.path('/es-modules/package-type-module/cjs.js') | 18 | | - ); | 19 | | - const basename = 'cjs.js'; | 20 | | - const child = spawn(process.execPath, [requiringCjsAsEsm]); | 21 | | - let stderr = ''; | 22 | | - child.stderr.setEncoding('utf8'); | 23 | | - child.stderr.on('data', (data) => { | 24 | | - stderr += data; | 25 | | - }); | 26 | | - child.on('close', common.mustCall((code, signal) => { | | 17 | + | | 18 | +describe('CJS ↔︎ ESM interop warnings', { concurrency: true }, () => { | | 19 | + | | 20 | + it(async () => { | | 21 | + const required = path.resolve( | | 22 | + fixtures.path('/es-modules/package-type-module/cjs.js') | | 23 | + ); | | 24 | + const basename = 'cjs.js'; | | 25 | + const { code, signal, stderr } = await spawnPromisified(execPath, [requiringCjsAsEsm]); | | 26 | + | | 27 | + assert.ok( | | 28 | + stderr.replaceAll('\r', '').includes( | | 29 | + `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${requiringCjsAsEsm} not supported.\n` | | 30 | + ) | | 31 | + ); | | 32 | + assert.ok( | | 33 | + stderr.replaceAll('\r', '').includes( | | 34 | + `Instead rename ${basename} to end in .cjs, change the requiring ` + | | 35 | + 'code to use dynamic import() which is available in all CommonJS ' + | | 36 | + `modules, or change "type": "module" to "type": "commonjs" in ${pjson} to ` + | | 37 | + 'treat all .js files as CommonJS (using .mjs for all ES modules ' + | | 38 | + 'instead).\n' | | 39 | + ) | | 40 | + ); | | 41 | + | 27 | 42 | assert.strictEqual(code, 1); | 28 | 43 | assert.strictEqual(signal, null); | | 44 | + }); | 29 | 45 |
| 30 | | - assert.ok(stderr.replaceAll('\r', '').includes( | 31 | | - `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${ | 32 | | - requiringCjsAsEsm} not supported.\n`)); | 33 | | - assert.ok(stderr.replaceAll('\r', '').includes( | 34 | | - `Instead rename ${basename} to end in .cjs, change the requiring ` + | 35 | | - 'code to use dynamic import() which is available in all CommonJS ' + | 36 | | - `modules, or change "type": "module" to "type": "commonjs" in ${pjson} to ` + | 37 | | - 'treat all .js files as CommonJS (using .mjs for all ES modules ' + | 38 | | - 'instead).\n')); | 39 | | - })); | 40 | | -} | | 46 | + it(async () => { | | 47 | + const required = path.resolve( | | 48 | + fixtures.path('/es-modules/package-type-module/esm.js') | | 49 | + ); | | 50 | + const basename = 'esm.js'; | | 51 | + const { code, signal, stderr } = await spawnPromisified(execPath, [requiringEsm]); | | 52 | + | | 53 | + assert.ok( | | 54 | + stderr.replace(/\r/g, '').includes( | | 55 | + `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${requiringEsm} not supported.\n` | | 56 | + ) | | 57 | + ); | | 58 | + assert.ok( | | 59 | + stderr.replace(/\r/g, '').includes( | | 60 | + `Instead change the require of ${basename} in ${requiringEsm} to` + | | 61 | + ' a dynamic import() which is available in all CommonJS modules.\n' | | 62 | + ) | | 63 | + ); | 41 | 64 |
| 42 | | -{ | 43 | | - const required = path.resolve( | 44 | | - fixtures.path('/es-modules/package-type-module/esm.js') | 45 | | - ); | 46 | | - const basename = 'esm.js'; | 47 | | - const child = spawn(process.execPath, [requiringEsm]); | 48 | | - let stderr = ''; | 49 | | - child.stderr.setEncoding('utf8'); | 50 | | - child.stderr.on('data', (data) => { | 51 | | - stderr += data; | 52 | | - }); | 53 | | - child.on('close', common.mustCall((code, signal) => { | 54 | 65 | assert.strictEqual(code, 1); | 55 | 66 | assert.strictEqual(signal, null); | 56 | | - | 57 | | - assert.ok(stderr.replace(/\r/g, '').includes( | 58 | | - `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${ | 59 | | - requiringEsm} not supported.\n`)); | 60 | | - assert.ok(stderr.replace(/\r/g, '').includes( | 61 | | - `Instead change the require of ${basename} in ${requiringEsm} to` + | 62 | | - ' a dynamic import() which is available in all CommonJS modules.\n')); | 63 | | - })); | 64 | | -} | | 67 | + }); | | 68 | +}); |
Collapse file Copy file name to clipboard +14-15Lines changed: 14 additions & 15 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | 'use strict'; | 2 | 2 |
| 3 | | -const common = require('../common'); | 4 | | -const fixtures = require('../common/fixtures'); | 5 | | -const { spawn } = require('child_process'); | 6 | | -const assert = require('assert'); | | 3 | +const { spawnPromisified } = require('../common'); | | 4 | +const fixtures = require('../common/fixtures.js'); | | 5 | +const assert = require('node:assert'); | | 6 | +const { execPath } = require('node:process'); | | 7 | +const { describe, it } = require('node:test'); | | 8 | + | 7 | 9 |
| 8 | 10 | const entry = fixtures.path('/es-modules/builtin-imports-case.mjs'); | 9 | 11 |
| 10 | | -const child = spawn(process.execPath, [entry]); | 11 | | -child.stderr.setEncoding('utf8'); | 12 | | -let stdout = ''; | 13 | | -child.stdout.setEncoding('utf8'); | 14 | | -child.stdout.on('data', (data) => { | 15 | | - stdout += data; | | 12 | +describe('ESM: importing builtins & CJS', () => { | | 13 | + it('should work', async () => { | | 14 | + const { code, signal, stdout } = await spawnPromisified(execPath, [entry]); | | 15 | + | | 16 | + assert.strictEqual(code, 0); | | 17 | + assert.strictEqual(signal, null); | | 18 | + assert.strictEqual(stdout, 'ok\n'); | | 19 | + }); | 16 | 20 | }); | 17 | | -child.on('close', common.mustCall((code, signal) => { | 18 | | - assert.strictEqual(code, 0); | 19 | | - assert.strictEqual(signal, null); | 20 | | - assert.strictEqual(stdout, 'ok\n'); | 21 | | -})); |
Collapse file Copy file name to clipboard +23-29Lines changed: 23 additions & 29 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | 'use strict'; | 2 | 2 |
| 3 | | -const common = require('../common'); | 4 | | -const fixtures = require('../common/fixtures'); | 5 | | -const { spawn } = require('child_process'); | 6 | | -const assert = require('assert'); | | 3 | +const { spawnPromisified } = require('../common'); | | 4 | +const fixtures = require('../common/fixtures.js'); | | 5 | +const assert = require('node:assert'); | | 6 | +const { execPath } = require('node:process'); | | 7 | +const { describe, it } = require('node:test'); | 7 | 8 |
| 8 | | -const entry = fixtures.path('/es-modules/cjs-exports.mjs'); | 9 | 9 |
| 10 | | -let child = spawn(process.execPath, [entry]); | 11 | | -child.stderr.setEncoding('utf8'); | 12 | | -let stdout = ''; | 13 | | -child.stdout.setEncoding('utf8'); | 14 | | -child.stdout.on('data', (data) => { | 15 | | - stdout += data; | 16 | | -}); | 17 | | -child.on('close', common.mustCall((code, signal) => { | 18 | | - assert.strictEqual(code, 0); | 19 | | - assert.strictEqual(signal, null); | 20 | | - assert.strictEqual(stdout, 'ok\n'); | 21 | | -})); | | 10 | +describe('ESM: importing CJS', { concurrency: true }, () => { | | 11 | + it('should support valid CJS exports', async () => { | | 12 | + const validEntry = fixtures.path('/es-modules/cjs-exports.mjs'); | | 13 | + const { code, signal, stdout } = await spawnPromisified(execPath, [validEntry]); | | 14 | + | | 15 | + assert.strictEqual(code, 0); | | 16 | + assert.strictEqual(signal, null); | | 17 | + assert.strictEqual(stdout, 'ok\n'); | | 18 | + }); | | 19 | + | | 20 | + it('should eror on invalid CJS exports', async () => { | | 21 | + const invalidEntry = fixtures.path('/es-modules/cjs-exports-invalid.mjs'); | | 22 | + const { code, signal, stderr } = await spawnPromisified(execPath, [invalidEntry]); | 22 | 23 |
| 23 | | -const entryInvalid = fixtures.path('/es-modules/cjs-exports-invalid.mjs'); | 24 | | -child = spawn(process.execPath, [entryInvalid]); | 25 | | -let stderr = ''; | 26 | | -child.stderr.setEncoding('utf8'); | 27 | | -child.stderr.on('data', (data) => { | 28 | | - stderr += data; | | 24 | + assert.strictEqual(code, 1); | | 25 | + assert.strictEqual(signal, null); | | 26 | + assert.ok(stderr.includes('Warning: To load an ES module')); | | 27 | + assert.ok(stderr.includes('Unexpected token \'export\'')); | | 28 | + }); | 29 | 29 | }); | 30 | | -child.on('close', common.mustCall((code, signal) => { | 31 | | - assert.strictEqual(code, 1); | 32 | | - assert.strictEqual(signal, null); | 33 | | - assert.ok(stderr.includes('Warning: To load an ES module')); | 34 | | - assert.ok(stderr.includes('Unexpected token \'export\'')); | 35 | | -})); |
|