esm: improve performance & tidy tests

lib/internal/modules/esm/loader.js

@@ -336,7 +336,7 @@ class ESMLoader {
    *  A list of exports from user-defined loaders (as returned by
    *  ESMLoader.import()).
    */
-  async addCustomLoaders(
+  addCustomLoaders(
     customLoaders = [],
   ) {
     for (let i = 0; i < customLoaders.length; i++) {

test/common/index.js

@@ -24,7 +24,7 @@
 const process = global.process;  // Some tests tamper with the process global.
 
 const assert = require('assert');
-const { exec, execSync, spawnSync } = require('child_process');
+const { exec, execSync, spawnSync, spawn: spawnAsync } = require('child_process');
 const fs = require('fs');
 // Do not require 'os' until needed so that test-os-checked-function can
 // monkey patch it. If 'os' is required here, that test will fail.
@@ -842,6 +842,36 @@ function requireNoPackageJSONAbove(dir = __dirname) {
   }
 }
 
+function spawnPromisified(...args) {
+  let stderr = '';
+  let stdout = '';
+
+  const child = spawnAsync(...args);
+  child.stderr.setEncoding('utf8');
+  child.stderr.on('data', (data) => { stderr += data; });
+  child.stdout.setEncoding('utf8');
+  child.stdout.on('data', (data) => { stdout += data; });
+
+  return new Promise((resolve, reject) => {
+    child.on('close', (code, signal) => {
+      resolve({
+        code,
+        signal,
+        stderr,
+        stdout,
+      });
+    });
+    child.on('error', (code, signal) => {
+      reject({
+        code,
+        signal,
+        stderr,
+        stdout,
+      });
+    });
+  });
+}
+
 const common = {
   allowGlobals,
   buildType,
@@ -891,6 +921,7 @@ const common = {
   skipIfEslintMissing,
   skipIfInspectorDisabled,
   skipIfWorker,
+  spawnPromisified,
 
   get enoughTestMem() {
     return require('os').totalmem() > 0x70000000; /* 1.75 Gb */

test/common/index.mjs

@@ -23,6 +23,7 @@ const {
   hasCrypto,
   hasIPv6,
   childShouldThrowAndAbort,
+  checkoutEOL,
   createZeroFilledFile,
   platformTimeout,
   allowGlobals,
@@ -47,7 +48,8 @@ const {
   getArrayBufferViews,
   getBufferSources,
   getTTYfd,
-  runWithInvalidFD
+  runWithInvalidFD,
+  spawnPromisified,
 } = common;
 
 export {
@@ -70,6 +72,7 @@ export {
   hasCrypto,
   hasIPv6,
   childShouldThrowAndAbort,
+  checkoutEOL,
   createZeroFilledFile,
   platformTimeout,
   allowGlobals,
@@ -95,5 +98,6 @@ export {
   getBufferSources,
   getTTYfd,
   runWithInvalidFD,
-  createRequire
+  createRequire,
+  spawnPromisified,
 };

test/es-module/test-cjs-esm-warn.js

@@ -1,64 +1,68 @@
 'use strict';
 
-const common = require('../common');
-const fixtures = require('../common/fixtures');
-const { spawn } = require('child_process');
-const assert = require('assert');
-const path = require('path');
+const { spawnPromisified } = require('../common');
+const fixtures = require('../common/fixtures.js');
+const assert = require('node:assert');
+const path = require('node:path');
+const { execPath } = require('node:process');
+const { describe, it } = require('node:test');
+
 
 const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
 const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js'));
 const pjson = path.resolve(
   fixtures.path('/es-modules/package-type-module/package.json')
 );
 
-{
-  const required = path.resolve(
-    fixtures.path('/es-modules/package-type-module/cjs.js')
-  );
-  const basename = 'cjs.js';
-  const child = spawn(process.execPath, [requiringCjsAsEsm]);
-  let stderr = '';
-  child.stderr.setEncoding('utf8');
-  child.stderr.on('data', (data) => {
-    stderr += data;
-  });
-  child.on('close', common.mustCall((code, signal) => {
+
+describe('CJS ↔︎ ESM interop warnings', { concurrency: true }, () => {
+
+  it(async () => {
+    const required = path.resolve(
+      fixtures.path('/es-modules/package-type-module/cjs.js')
+    );
+    const basename = 'cjs.js';
+    const { code, signal, stderr } = await spawnPromisified(execPath, [requiringCjsAsEsm]);
+
+    assert.ok(
+      stderr.replaceAll('\r', '').includes(
+        `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${requiringCjsAsEsm} not supported.\n`
+      )
+    );
+    assert.ok(
+      stderr.replaceAll('\r', '').includes(
+        `Instead rename ${basename} to end in .cjs, change the requiring ` +
+        'code to use dynamic import() which is available in all CommonJS ' +
+        `modules, or change "type": "module" to "type": "commonjs" in ${pjson} to ` +
+        'treat all .js files as CommonJS (using .mjs for all ES modules ' +
+        'instead).\n'
+      )
+    );
+
     assert.strictEqual(code, 1);
     assert.strictEqual(signal, null);
+  });
 
-    assert.ok(stderr.replaceAll('\r', '').includes(
-      `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${
-        requiringCjsAsEsm} not supported.\n`));
-    assert.ok(stderr.replaceAll('\r', '').includes(
-      `Instead rename ${basename} to end in .cjs, change the requiring ` +
-      'code to use dynamic import() which is available in all CommonJS ' +
-      `modules, or change "type": "module" to "type": "commonjs" in ${pjson} to ` +
-      'treat all .js files as CommonJS (using .mjs for all ES modules ' +
-      'instead).\n'));
-  }));
-}
+  it(async () => {
+    const required = path.resolve(
+      fixtures.path('/es-modules/package-type-module/esm.js')
+    );
+    const basename = 'esm.js';
+    const { code, signal, stderr } = await spawnPromisified(execPath, [requiringEsm]);
+
+    assert.ok(
+      stderr.replace(/\r/g, '').includes(
+        `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${requiringEsm} not supported.\n`
+      )
+    );
+    assert.ok(
+      stderr.replace(/\r/g, '').includes(
+        `Instead change the require of ${basename} in ${requiringEsm} to` +
+        ' a dynamic import() which is available in all CommonJS modules.\n'
+      )
+    );
 
-{
-  const required = path.resolve(
-    fixtures.path('/es-modules/package-type-module/esm.js')
-  );
-  const basename = 'esm.js';
-  const child = spawn(process.execPath, [requiringEsm]);
-  let stderr = '';
-  child.stderr.setEncoding('utf8');
-  child.stderr.on('data', (data) => {
-    stderr += data;
-  });
-  child.on('close', common.mustCall((code, signal) => {
     assert.strictEqual(code, 1);
     assert.strictEqual(signal, null);
-
-    assert.ok(stderr.replace(/\r/g, '').includes(
-      `Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${
-        requiringEsm} not supported.\n`));
-    assert.ok(stderr.replace(/\r/g, '').includes(
-      `Instead change the require of ${basename} in ${requiringEsm} to` +
-      ' a dynamic import() which is available in all CommonJS modules.\n'));
-  }));
-}
+  });
+});

test/es-module/test-esm-cjs-builtins.js

@@ -1,21 +1,20 @@
 'use strict';
 
-const common = require('../common');
-const fixtures = require('../common/fixtures');
-const { spawn } = require('child_process');
-const assert = require('assert');
+const { spawnPromisified } = require('../common');
+const fixtures = require('../common/fixtures.js');
+const assert = require('node:assert');
+const { execPath } = require('node:process');
+const { describe, it } = require('node:test');
+
 
 const entry = fixtures.path('/es-modules/builtin-imports-case.mjs');
 
-const child = spawn(process.execPath, [entry]);
-child.stderr.setEncoding('utf8');
-let stdout = '';
-child.stdout.setEncoding('utf8');
-child.stdout.on('data', (data) => {
-  stdout += data;
+describe('ESM: importing builtins & CJS', () => {
+  it('should work', async () => {
+    const { code, signal, stdout } = await spawnPromisified(execPath, [entry]);
+
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+    assert.strictEqual(stdout, 'ok\n');
+  });
 });
-child.on('close', common.mustCall((code, signal) => {
-  assert.strictEqual(code, 0);
-  assert.strictEqual(signal, null);
-  assert.strictEqual(stdout, 'ok\n');
-}));

test/es-module/test-esm-cjs-exports.js

@@ -1,35 +1,29 @@
 'use strict';
 
-const common = require('../common');
-const fixtures = require('../common/fixtures');
-const { spawn } = require('child_process');
-const assert = require('assert');
+const { spawnPromisified } = require('../common');
+const fixtures = require('../common/fixtures.js');
+const assert = require('node:assert');
+const { execPath } = require('node:process');
+const { describe, it } = require('node:test');
 
-const entry = fixtures.path('/es-modules/cjs-exports.mjs');
 
-let child = spawn(process.execPath, [entry]);
-child.stderr.setEncoding('utf8');
-let stdout = '';
-child.stdout.setEncoding('utf8');
-child.stdout.on('data', (data) => {
-  stdout += data;
-});
-child.on('close', common.mustCall((code, signal) => {
-  assert.strictEqual(code, 0);
-  assert.strictEqual(signal, null);
-  assert.strictEqual(stdout, 'ok\n');
-}));
+describe('ESM: importing CJS', { concurrency: true }, () => {
+  it('should support valid CJS exports', async () => {
+    const validEntry = fixtures.path('/es-modules/cjs-exports.mjs');
+    const { code, signal, stdout } = await spawnPromisified(execPath, [validEntry]);
+
+    assert.strictEqual(code, 0);
+    assert.strictEqual(signal, null);
+    assert.strictEqual(stdout, 'ok\n');
+  });
+
+  it('should eror on invalid CJS exports', async () => {
+    const invalidEntry = fixtures.path('/es-modules/cjs-exports-invalid.mjs');
+    const { code, signal, stderr } = await spawnPromisified(execPath, [invalidEntry]);
 
-const entryInvalid = fixtures.path('/es-modules/cjs-exports-invalid.mjs');
-child = spawn(process.execPath, [entryInvalid]);
-let stderr = '';
-child.stderr.setEncoding('utf8');
-child.stderr.on('data', (data) => {
-  stderr += data;
+    assert.strictEqual(code, 1);
+    assert.strictEqual(signal, null);
+    assert.ok(stderr.includes('Warning: To load an ES module'));
+    assert.ok(stderr.includes('Unexpected token \'export\''));
+  });
 });
-child.on('close', common.mustCall((code, signal) => {
-  assert.strictEqual(code, 1);
-  assert.strictEqual(signal, null);
-  assert.ok(stderr.includes('Warning: To load an ES module'));
-  assert.ok(stderr.includes('Unexpected token \'export\''));
-}));