test: fix some assumptions in tests

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: nodejs#48958 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Michaël Zasso <targos@protonmail.com>
RafaelGSS · Aug 15, 2023 · 6b07b68 · 6b07b68
1 parent 9e33a36
commit 6b07b68
Show file tree

Hide file tree

Showing 10 changed files with 33 additions and 31 deletions.
diff --git a/test/es-module/test-cjs-legacyMainResolve-permission.js b/test/es-module/test-cjs-legacyMainResolve-permission.js
@@ -49,21 +49,21 @@ describe('legacyMainResolve', () => {
 
             const packageJsonUrl = pathToFileURL(
               path.resolve(
-                '${fixtextureFolderEscaped}',
+                ${JSON.stringify(fixtextureFolderEscaped)},
                 'package.json'
               )
             );
 
             const packageConfig = { main: '${mainOrFolder}' };
             const base = path.resolve(
-              '${fixtextureFolderEscaped}'
+              ${JSON.stringify(fixtextureFolderEscaped)},
             );
 
             assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
               code: 'ERR_ACCESS_DENIED',
               resource: path.resolve(
-                '${fixtextureFolderEscaped}',
-                '${mainOrFolder}'
+                ${JSON.stringify(fixtextureFolderEscaped)},
+                ${JSON.stringify(mainOrFolder)},
               )
             });
           `,
@@ -103,23 +103,23 @@ describe('legacyMainResolve', () => {
 
             const packageJsonUrl = pathToFileURL(
               path.resolve(
-                '${fixtextureFolderEscaped}',
-                '${folder}',
+                ${JSON.stringify(fixtextureFolderEscaped)},
+                ${JSON.stringify(folder)},
                 'package.json'
               )
             );
 
             const packageConfig = { main: undefined };
             const base = path.resolve(
-              '${fixtextureFolderEscaped}'
+              ${JSON.stringify(fixtextureFolderEscaped)},
             );
 
             assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
               code: 'ERR_ACCESS_DENIED',
               resource: path.resolve(
-                '${fixtextureFolderEscaped}',
-                '${folder}',
-                '${expectedFile}'
+                ${JSON.stringify(fixtextureFolderEscaped)},
+                ${JSON.stringify(folder)},
+                ${JSON.stringify(expectedFile)},
               )
             });
           `,

diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js
@@ -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) => {
@@ -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'));

diff --git a/test/es-module/test-esm-loader-spawn-promisified.mjs b/test/es-module/test-esm-loader-spawn-promisified.mjs
@@ -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/);
@@ -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'),
@@ -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');
@@ -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;
@@ -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);
       });`,

diff --git a/test/parallel/test-fs-mkdtemp.js b/test/parallel/test-fs-mkdtemp.js
@@ -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();
@@ -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.'));

diff --git a/test/parallel/test-internal-util-decorate-error-stack.js b/test/parallel/test-internal-util-decorate-error-stack.js
@@ -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);

diff --git a/test/parallel/test-repl-require-context.js b/test/parallel/test-repl-require-context.js
@@ -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();
diff --git a/test/parallel/test-stdio-pipe-stderr.js b/test/parallel/test-stdio-pipe-stderr.js
@@ -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();

diff --git a/test/parallel/test-trace-events-worker-metadata-with-name.js b/test/parallel/test-trace-events-worker-metadata-with-name.js
@@ -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();

diff --git a/test/parallel/test-trace-events-worker-metadata.js b/test/parallel/test-trace-events-worker-metadata.js
@@ -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();

diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs
@@ -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)
@@ -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 });
@@ -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 });
@@ -276,7 +277,7 @@ console.log(values.random);
 
   it('should not load --import modules in main process', 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 });
 
@@ -318,9 +319,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 });
   });