From 5c325b3fbf01bdd25eaa55a5fbe328017584a70f Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 31 Jul 2023 15:09:07 +0200 Subject: [PATCH] test: fix some assumptions in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/48958 Reviewed-By: Moshe Atlow Reviewed-By: Michaël Zasso --- .../test-cjs-legacyMainResolve-permission.js | 20 +++++++++---------- test/es-module/test-esm-dynamic-import.js | 8 ++++---- .../test-esm-loader-spawn-promisified.mjs | 10 +++++----- test/parallel/test-fs-mkdtemp.js | 3 ++- ...test-internal-util-decorate-error-stack.js | 2 +- test/parallel/test-repl-require-context.js | 2 +- test/parallel/test-stdio-pipe-stderr.js | 2 +- ...-trace-events-worker-metadata-with-name.js | 2 +- .../test-trace-events-worker-metadata.js | 2 +- test/sequential/test-watch-mode.mjs | 13 ++++++------ 10 files changed, 33 insertions(+), 31 deletions(-) diff --git a/test/es-module/test-cjs-legacyMainResolve-permission.js b/test/es-module/test-cjs-legacyMainResolve-permission.js index cf54685736c099..9f78883eebb7cd 100644 --- 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 index ac6b35ebc1bc15..d246841c2a6d8b 100644 --- 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 index 3a107ea64816b2..162316ade410b9 100644 --- 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 index b7bb0bbafe0581..9ebf8880d23732 100644 --- 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 index 3566d9375fb81c..f3034fbb05ae54 100644 --- 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 index 750235818b8bfc..af09249c2de919 100644 --- 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 index 9ec41b4159fdf6..1737424bb049fc 100644 --- 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 index 6c3a44f9566d9c..bf6e1005aa458f 100644 --- 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 index 8b4d0be9c60713..6a8702ccadbc7b 100644 --- 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 index 17063195fa9e5e..38654a78a1dc7f 100644 --- 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 }); });