Skip to content

Commit

Permalink
test: fix some assumptions in tests
Browse files Browse the repository at this point in the history
Some tests are assuming they will be run from a directory that do not
contain any quote or special character in its path. That assumption is
not necessary, using `JSON.stringify` or `pathToFileURL` ensures the
test can be run whatever the path looks like.

PR-URL: #48958
Backport-PR-URL: #50669
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
aduh95 authored and targos committed Nov 23, 2023
1 parent 48c25b1 commit adb9280
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 21 deletions.
8 changes: 4 additions & 4 deletions test/es-module/test-esm-dynamic-import.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
'use strict';
const common = require('../common');
const { pathToFileURL } = require('url');
const assert = require('assert');

const relativePath = '../fixtures/es-modules/test-esm-ok.mjs';
const absolutePath = require.resolve('../fixtures/es-modules/test-esm-ok.mjs');
const targetURL = new URL('file:///');
targetURL.pathname = absolutePath;
const absolutePath = require.resolve(relativePath);
const targetURL = pathToFileURL(absolutePath);

function expectModuleError(result, code, message) {
Promise.resolve(result).catch(common.mustCall((error) => {
Expand Down Expand Up @@ -41,7 +41,7 @@ function expectFsNamespace(result) {
// expectOkNamespace(import(relativePath));
expectOkNamespace(eval(`import("${relativePath}")`));
expectOkNamespace(eval(`import("${relativePath}")`));
expectOkNamespace(eval(`import("${targetURL}")`));
expectOkNamespace(eval(`import(${JSON.stringify(targetURL)})`));

// Importing a built-in, both direct & via eval
expectFsNamespace(import('fs'));
Expand Down
10 changes: 5 additions & 5 deletions test/es-module/test-esm-loader-spawn-promisified.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Loader hooks throwing errors', { concurrency: true }, () => {
fixtures.fileURL('/es-module-loaders/hooks-custom.mjs'),
'--input-type=module',
'--eval',
`import '${fixtures.fileURL('/es-modules/file.unknown')}'`,
`import ${JSON.stringify(fixtures.fileURL('/es-modules/file.unknown'))}`,
]);

assert.match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/);
Expand Down Expand Up @@ -142,7 +142,7 @@ describe('Loader hooks throwing errors', { concurrency: true }, () => {
`import assert from 'node:assert';
await Promise.allSettled([
import('nonexistent/file.mjs'),
import('${fixtures.fileURL('/es-modules/file.unknown')}'),
import(${JSON.stringify(fixtures.fileURL('/es-modules/file.unknown'))}),
import('esmHook/badReturnVal.mjs'),
import('esmHook/format.false'),
import('esmHook/format.true'),
Expand Down Expand Up @@ -170,7 +170,7 @@ describe('Loader hooks parsing modules', { concurrency: true }, () => {
'--input-type=module',
'--eval',
`import assert from 'node:assert';
await import('${fixtures.fileURL('/es-module-loaders/js-as-esm.js')}')
await import(${JSON.stringify(fixtures.fileURL('/es-module-loaders/js-as-esm.js'))})
.then((parsedModule) => {
assert.strictEqual(typeof parsedModule, 'object');
assert.strictEqual(parsedModule.namedExport, 'named-export');
Expand All @@ -191,7 +191,7 @@ describe('Loader hooks parsing modules', { concurrency: true }, () => {
'--input-type=module',
'--eval',
`import assert from 'node:assert';
await import('${fixtures.fileURL('/es-modules/file.ext')}')
await import(${JSON.stringify(fixtures.fileURL('/es-modules/file.ext'))})
.then((parsedModule) => {
assert.strictEqual(typeof parsedModule, 'object');
const { default: defaultExport } = parsedModule;
Expand Down Expand Up @@ -258,7 +258,7 @@ describe('Loader hooks parsing modules', { concurrency: true }, () => {
'--input-type=module',
'--eval',
`import assert from 'node:assert';
await import('${fixtures.fileURL('/es-modules/stateful.mjs')}')
await import(${JSON.stringify(fixtures.fileURL('/es-modules/stateful.mjs'))})
.then(({ default: count }) => {
assert.strictEqual(count(), 1);
});`,
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-fs-mkdtemp.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const { pathToFileURL } = require('url');

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
Expand Down Expand Up @@ -40,7 +41,7 @@ function handler(err, folder) {

// Test with URL object
{
tmpdir.url = new URL(`file://${tmpdir.path}`);
tmpdir.url = pathToFileURL(tmpdir.path);
const urljoin = (base, path) => new URL(path, base);

const tmpFolder = fs.mkdtempSync(urljoin(tmpdir.url, 'foo.'));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-internal-util-decorate-error-stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ checkStack(err.stack);
// Verify that the stack is only decorated once for uncaught exceptions.
const args = [
'-e',
`require('${badSyntaxPath}')`,
`require(${JSON.stringify(badSyntaxPath)})`,
];
const result = spawnSync(process.argv[0], args, { encoding: 'utf8' });
checkStack(result.stderr);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-repl-require-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ child.on('exit', common.mustCall(() => {

child.stdin.write('const isObject = (obj) => obj.constructor === Object;\n');
child.stdin.write('isObject({});\n');
child.stdin.write(`require('${fixture}').isObject({});\n`);
child.stdin.write(`require(${JSON.stringify(fixture)}).isObject({});\n`);
child.stdin.write('.exit');
child.stdin.end();
2 changes: 1 addition & 1 deletion test/parallel/test-stdio-pipe-stderr.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fs.writeFileSync(fakeModulePath, '', 'utf8');

stream.on('open', () => {
spawnSync(process.execPath, {
input: `require("${fakeModulePath.replace(/\\/g, '/')}")`,
input: `require(${JSON.stringify(fakeModulePath)})`,
stdio: ['pipe', 'pipe', stream]
});
const stderr = fs.readFileSync(stderrOutputPath, 'utf8').trim();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { isMainThread } = require('worker_threads');

if (isMainThread) {
const CODE = 'const { Worker } = require(\'worker_threads\'); ' +
`new Worker('${__filename.replace(/\\/g, '/')}', { name: 'foo' })`;
`new Worker(${JSON.stringify(__filename)}, { name: 'foo' })`;
const FILE_NAME = 'node_trace.1.log';
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-trace-events-worker-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { isMainThread } = require('worker_threads');

if (isMainThread) {
const CODE = 'const { Worker } = require(\'worker_threads\'); ' +
`new Worker('${__filename.replace(/\\/g, '/')}')`;
`new Worker(${JSON.stringify(__filename)})`;
const FILE_NAME = 'node_trace.1.log';
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
Expand Down
13 changes: 7 additions & 6 deletions test/sequential/test-watch-mode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { describe, it } from 'node:test';
import { spawn } from 'node:child_process';
import { writeFileSync, readFileSync, mkdirSync } from 'node:fs';
import { inspect } from 'node:util';
import { pathToFileURL } from 'node:url';
import { createInterface } from 'node:readline';

if (common.isIBMi)
Expand Down Expand Up @@ -188,7 +189,7 @@ console.log("don't show me");`);
it('should watch changes to dependencies - cjs', async () => {
const dependency = createTmpFile('module.exports = {};');
const file = createTmpFile(`
const dependency = require('${dependency.replace(/\\/g, '/')}');
const dependency = require(${JSON.stringify(dependency)});
console.log(dependency);
`);
const { stderr, stdout } = await runWriteSucceed({ file, watchedFile: dependency });
Expand All @@ -206,7 +207,7 @@ console.log(dependency);
it('should watch changes to dependencies - esm', async () => {
const dependency = createTmpFile('module.exports = {};');
const file = createTmpFile(`
import dependency from 'file://${dependency.replace(/\\/g, '/')}';
import dependency from ${JSON.stringify(pathToFileURL(dependency))};
console.log(dependency);
`, '.mjs');
const { stderr, stdout } = await runWriteSucceed({ file, watchedFile: dependency });
Expand Down Expand Up @@ -278,7 +279,7 @@ console.log(values.random);
skip: 'enable once --import is backported',
}, async () => {
const file = createTmpFile();
const imported = `file://${createTmpFile('setImmediate(() => process.exit(0));')}`;
const imported = pathToFileURL(createTmpFile('setImmediate(() => process.exit(0));'));
const args = ['--import', imported, file];
const { stderr, stdout } = await runWriteSucceed({ file, watchedFile: file, args });

Expand Down Expand Up @@ -320,9 +321,9 @@ console.log(values.random);
it('should watch changes to previously missing ESM dependency', {
skip: !supportsRecursive
}, async () => {
const dependency = path.join(tmpdir.path, `${tmpFiles++}.mjs`);
const relativeDependencyPath = `./${path.basename(dependency)}`;
const dependant = createTmpFile(`import '${relativeDependencyPath}'`, '.mjs');
const relativeDependencyPath = `./${tmpFiles++}.mjs`;
const dependency = path.join(tmpdir.path, relativeDependencyPath);
const dependant = createTmpFile(`import ${JSON.stringify(relativeDependencyPath)}`, '.mjs');

await failWriteSucceed({ file: dependant, watchedFile: dependency });
});
Expand Down

0 comments on commit adb9280

Please sign in to comment.