Skip to content

Commit b19564b

Browse files
JakobJingleheimerruyadorno
JakobJingleheimer
authored and
ruyadorno
committed
test: refactor ESM tests to improve performance
PR-URL: #43784 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 728e18e commit b19564b

File tree

56 files changed

+1390
-1442
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+1390
-1442
lines changed

‎lib/internal/modules/esm/loader.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ class ESMLoader {
336336
* A list of exports from user-defined loaders (as returned by
337337
* ESMLoader.import()).
338338
*/
339-
async addCustomLoaders(
339+
addCustomLoaders(
340340
customLoaders = [],
341341
) {
342342
for (let i = 0; i < customLoaders.length; i++) {

‎test/common/index.js

+32-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
const process = global.process; // Some tests tamper with the process global.
2525

2626
const assert = require('assert');
27-
const { exec, execSync, spawnSync } = require('child_process');
27+
const { exec, execSync, spawn, spawnSync } = require('child_process');
2828
const fs = require('fs');
2929
// Do not require 'os' until needed so that test-os-checked-function can
3030
// monkey patch it. If 'os' is required here, that test will fail.
@@ -842,6 +842,36 @@ function requireNoPackageJSONAbove(dir = __dirname) {
842842
}
843843
}
844844

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+
845875
const common = {
846876
allowGlobals,
847877
buildType,
@@ -891,6 +921,7 @@ const common = {
891921
skipIfEslintMissing,
892922
skipIfInspectorDisabled,
893923
skipIfWorker,
924+
spawnPromisified,
894925

895926
get enoughTestMem() {
896927
return require('os').totalmem() > 0x70000000; /* 1.75 Gb */

‎test/common/index.mjs

+6-2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const {
2323
hasCrypto,
2424
hasIPv6,
2525
childShouldThrowAndAbort,
26+
checkoutEOL,
2627
createZeroFilledFile,
2728
platformTimeout,
2829
allowGlobals,
@@ -47,7 +48,8 @@ const {
4748
getArrayBufferViews,
4849
getBufferSources,
4950
getTTYfd,
50-
runWithInvalidFD
51+
runWithInvalidFD,
52+
spawnPromisified,
5153
} = common;
5254

5355
export {
@@ -70,6 +72,7 @@ export {
7072
hasCrypto,
7173
hasIPv6,
7274
childShouldThrowAndAbort,
75+
checkoutEOL,
7376
createZeroFilledFile,
7477
platformTimeout,
7578
allowGlobals,
@@ -95,5 +98,6 @@ export {
9598
getBufferSources,
9699
getTTYfd,
97100
runWithInvalidFD,
98-
createRequire
101+
createRequire,
102+
spawnPromisified,
99103
};

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

+53-49
Original file line numberDiff line numberDiff line change
@@ -1,64 +1,68 @@
11
'use strict';
22

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+
810

911
const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
1012
const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js'));
1113
const pjson = path.resolve(
1214
fixtures.path('/es-modules/package-type-module/package.json')
1315
);
1416

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+
2742
assert.strictEqual(code, 1);
2843
assert.strictEqual(signal, null);
44+
});
2945

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+
);
4164

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) => {
5465
assert.strictEqual(code, 1);
5566
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+
});

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

+14-15
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,20 @@
11
'use strict';
22

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+
79

810
const entry = fixtures.path('/es-modules/builtin-imports-case.mjs');
911

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+
});
1620
});
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-
}));

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

+23-29
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,29 @@
11
'use strict';
22

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');
78

8-
const entry = fixtures.path('/es-modules/cjs-exports.mjs');
99

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]);
2223

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+
});
2929
});
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-
}));

0 commit comments

Comments
 (0)