diff --git a/tests/config/install-prettier.js b/tests/config/install-prettier.js index 5a2054e26..2fac432d0 100644 --- a/tests/config/install-prettier.js +++ b/tests/config/install-prettier.js @@ -27,7 +27,7 @@ if (!allowedClients.has(client)) { const directoriesToClean = new Set(); -process.on("exit", cleanUp); +process.once("exit", cleanUp); function cleanUp() { if (directoriesToClean.size === 0) { diff --git a/tests/integration/__tests__/config-file-typescript.js b/tests/integration/__tests__/config-file-typescript.js index 585ea19ac..d1962f3ae 100644 --- a/tests/integration/__tests__/config-file-typescript.js +++ b/tests/integration/__tests__/config-file-typescript.js @@ -59,12 +59,10 @@ if (NODE_JS_MAJOR_VERSION >= 22) { } if (NODE_JS_MAJOR_VERSION === 22) { - test("Should throw errors when flags are missing", async () => { - await runCli( - "cli/config/ts/auto-discovery/", - ["--stdin-filepath", "foo.js"], - { input: code }, - ).test({ + test("Should throw errors when flags are missing", () => { + runCli("cli/config/ts/auto-discovery/", ["--stdin-filepath", "foo.js"], { + input: code, + }).test({ status: "non-zero", stdout: "", write: [], @@ -74,26 +72,24 @@ if (NODE_JS_MAJOR_VERSION === 22) { } if (NODE_JS_MAJOR_VERSION < 22) { - test("Should throw errors when Node.js < 22", async () => { - await runCli( - "cli/config/ts/auto-discovery/", - ["--stdin-filepath", "foo.js"], - { input: code }, - ).test({ + test("Should throw errors when Node.js < 22", () => { + runCli("cli/config/ts/auto-discovery/", ["--stdin-filepath", "foo.js"], { + input: code, + }).test({ status: "non-zero", stdout: "", write: [], stderr: expect.stringMatching(/Unknown file extension ".ts" for/u), }); - // https://github.com/nodejs/node/issues/41103 - await expect(() => - runCli("cli/config/ts/auto-discovery/", ["--stdin-filepath", "foo.js"], { - input: code, - nodeOptions: NODE_TS_SUPPORT_FLAGS, - }), - ).rejects.toThrow( - /Initiated Worker with invalid execArgv flags: --experimental-strip-types/u, - ); + runCli("cli/config/ts/auto-discovery/", ["--stdin-filepath", "foo.js"], { + input: code, + nodeOptions: NODE_TS_SUPPORT_FLAGS, + }).test({ + status: "non-zero", + stdout: "", + write: [], + stderr: expect.stringMatching(/bad option: --experimental-strip-types/u), + }); }); } diff --git a/tests/integration/__tests__/patterns-dirs.js b/tests/integration/__tests__/patterns-dirs.js index 2a31ef1f1..95da23000 100644 --- a/tests/integration/__tests__/patterns-dirs.js +++ b/tests/integration/__tests__/patterns-dirs.js @@ -129,41 +129,61 @@ describe("plugins `*`", () => { if (path.sep === "/") { // Don't use snapshots in these tests as they're conditionally executed on non-Windows only. - const base = path.resolve(__dirname, "../cli/patterns-dirs"); + const base = path.resolve(__dirname, "../cli/patterns-backslashes"); + const directoryA = path.join(base, "test-a\\"); + const directoryB = path.join(base, String.raw`test-b\?`); + + fs.rmSync(directoryA, { force: true, recursive: true }); + fs.rmSync(directoryB, { force: true, recursive: true }); + for (const directory of [directoryA, directoryB]) { + fs.mkdirSync(directory); + fs.writeFileSync(path.join(directory, "test.js"), "x"); + } describe("Backslashes in names", () => { - // We can't commit these dirs without causing problems on Windows. + testPatterns( + "", + [String.raw`test-a\/test.js`], + { + stdout: String.raw`test-a\/test.js`, + }, + base, + ); + testPatterns( + "", + ["test-a\\"], + { stdout: String.raw`test-a\/test.js` }, + base, + ); + testPatterns( + "", + ["test-a*/*"], + { stdout: String.raw`test-a\/test.js` }, + base, + ); - beforeAll(() => { - fs.mkdirSync(path.resolve(base, "test-a\\")); - fs.writeFileSync(path.resolve(base, "test-a\\", "test.js"), "x"); - fs.mkdirSync(path.resolve(base, String.raw`test-b\?`)); - fs.writeFileSync( - path.resolve(base, String.raw`test-b\?`, "test.js"), - "x", - ); - }); - - afterAll(() => { - fs.unlinkSync(path.resolve(base, "test-a\\", "test.js")); - fs.rmdirSync(path.resolve(base, "test-a\\")); - fs.unlinkSync(path.resolve(base, String.raw`test-b\?`, "test.js")); - fs.rmdirSync(path.resolve(base, String.raw`test-b\?`)); - }); - - testPatterns("", [String.raw`test-a\/test.js`], { - stdout: String.raw`test-a\/test.js`, - }); - testPatterns("", ["test-a\\"], { stdout: String.raw`test-a\/test.js` }); - testPatterns("", ["test-a*/*"], { stdout: String.raw`test-a\/test.js` }); - - testPatterns("", [String.raw`test-b\?/test.js`], { - stdout: String.raw`test-b\?/test.js`, - }); - testPatterns("", [String.raw`test-b\?`], { - stdout: String.raw`test-b\?/test.js`, - }); - testPatterns("", ["test-b*/*"], { stdout: String.raw`test-b\?/test.js` }); + testPatterns( + "", + [String.raw`test-b\?/test.js`], + { + stdout: String.raw`test-b\?/test.js`, + }, + base, + ); + testPatterns( + "", + [String.raw`test-b\?`], + { + stdout: String.raw`test-b\?/test.js`, + }, + base, + ); + testPatterns( + "", + ["test-b*/*"], + { stdout: String.raw`test-b\?/test.js` }, + base, + ); }); } @@ -191,28 +211,23 @@ function isSymlinkSupported() { const base = path.join(__dirname, "../cli/patterns-symlinks"); const directoryA = path.join(base, "test-a"); const directoryB = path.join(base, "test-b"); - const clean = () => { - fs.rmSync(directoryA, { force: true, recursive: true }); - fs.rmSync(directoryB, { force: true, recursive: true }); - }; - beforeAll(() => { - clean(); - fs.mkdirSync(directoryA); - fs.mkdirSync(directoryB); - fs.writeFileSync(path.join(directoryA, "a.js"), "x"); - fs.writeFileSync(path.join(directoryB, "b.js"), "x"); - fs.symlinkSync(directoryA, path.join(directoryA, "symlink-to-directory-a")); - fs.symlinkSync(directoryB, path.join(directoryA, "symlink-to-directory-b")); - fs.symlinkSync( - path.join(directoryA, "a.js"), - path.join(directoryA, "symlink-to-file-a"), - ); - fs.symlinkSync( - path.join(directoryB, "b.js"), - path.join(directoryA, "symlink-to-file-b"), - ); - }); - afterAll(clean); + + fs.rmSync(directoryA, { force: true, recursive: true }); + fs.rmSync(directoryB, { force: true, recursive: true }); + fs.mkdirSync(directoryA); + fs.mkdirSync(directoryB); + fs.writeFileSync(path.join(directoryA, "a.js"), "x"); + fs.writeFileSync(path.join(directoryB, "b.js"), "x"); + fs.symlinkSync(directoryA, path.join(directoryA, "symlink-to-directory-a")); + fs.symlinkSync(directoryB, path.join(directoryA, "symlink-to-directory-b")); + fs.symlinkSync( + path.join(directoryA, "a.js"), + path.join(directoryA, "symlink-to-file-a"), + ); + fs.symlinkSync( + path.join(directoryB, "b.js"), + path.join(directoryA, "symlink-to-file-b"), + ); test("file struct", async () => { const getFileStruct = async (directory) => diff --git a/tests/integration/cli-worker.js b/tests/integration/cli-worker.js index 79d7cff75..217e136e5 100644 --- a/tests/integration/cli-worker.js +++ b/tests/integration/cli-worker.js @@ -2,7 +2,6 @@ import fs from "node:fs"; import path from "node:path"; import readline from "node:readline"; import url from "node:url"; -import { parentPort, workerData } from "node:worker_threads"; import { prettierCli, prettierMainEntry } from "./env.js"; const normalizeToPosix = @@ -19,9 +18,7 @@ const replaceAll = (text, find, replacement) => ? text.replaceAll(find, replacement) : text.split(find).join(replacement); -async function run() { - const { options } = workerData; - +async function run(options) { Date.now = () => 0; /* @@ -78,7 +75,7 @@ async function run() { ); } - parentPort.postMessage({ + process.send({ action: "write-file", data: { filename, content }, }); @@ -88,21 +85,22 @@ async function run() { await promise; } -parentPort.on("message", async () => { - const originalExit = process.exit; - - // https://github.com/nodejs/node/issues/30491 - process.stdout.cork(); - process.stderr.cork(); - process.exit = (code) => { - process.stdout.end(); - process.stderr.end(); - originalExit(code ?? process.exitCode ?? 0); - }; - +process.once("message", async (data) => { try { - await run(); + await run(data); } finally { + // On MacOS, if we exit too quick the stdio won't received on main thread + if (process.platform === "darwin") { + await Promise.all( + ["stdout", "stderr"].map( + (stream) => + new Promise((resolve) => { + process[stream].once("finish", resolve); + process[stream].end(); + }), + ), + ); + } process.exit(); } }); diff --git a/tests/integration/cli/patterns-backslashes/.gitignore b/tests/integration/cli/patterns-backslashes/.gitignore new file mode 100644 index 000000000..96fef51b5 --- /dev/null +++ b/tests/integration/cli/patterns-backslashes/.gitignore @@ -0,0 +1,2 @@ +# dynamically created by tests +* diff --git a/tests/integration/cli/patterns-symlinks/.gitignore b/tests/integration/cli/patterns-symlinks/.gitignore index e53c48ff5..96fef51b5 100644 --- a/tests/integration/cli/patterns-symlinks/.gitignore +++ b/tests/integration/cli/patterns-symlinks/.gitignore @@ -1,2 +1,2 @@ # dynamically created by tests -test-* +* diff --git a/tests/integration/run-cli.js b/tests/integration/run-cli.js index ec580dce4..69ae56cf3 100644 --- a/tests/integration/run-cli.js +++ b/tests/integration/run-cli.js @@ -1,29 +1,16 @@ +import childProcess from "node:child_process"; import path from "node:path"; import url from "node:url"; -import { Worker } from "node:worker_threads"; -const CLI_WORKER_FILE = new URL("./cli-worker.js", import.meta.url); +// Though the doc says `childProcess.fork` accepts `URL`, but seems not true +// TODO: Use `URL` directly when we drop support for Node.js v14 +const CLI_WORKER_FILE = url.fileURLToPath( + new URL("./cli-worker.js", import.meta.url), +); const INTEGRATION_TEST_DIRECTORY = url.fileURLToPath( new URL("./", import.meta.url), ); -const streamToString = (stream) => - new Promise((resolve, reject) => { - let result = ""; - - stream.on("data", (data) => { - result += data.toString(); - }); - - stream.on("end", () => { - resolve(result); - }); - - stream.on("error", (error) => { - reject(error); - }); - }); - const removeFinalNewLine = (string) => string.endsWith("\n") ? string.slice(0, -1) : string; @@ -39,9 +26,8 @@ function runCliWorker(dir, args, options) { }; const nodeOptions = options?.nodeOptions ?? []; - - const worker = new Worker(CLI_WORKER_FILE, { - argv: args, + const worker = childProcess.fork(CLI_WORKER_FILE, args, { + cwd: dir, execArgv: [ "--trace-deprecation", ...(SUPPORTS_DISABLE_WARNING_FLAG @@ -49,18 +35,13 @@ function runCliWorker(dir, args, options) { : []), ...nodeOptions, ], - stdout: true, - stderr: true, + stdio: "pipe", env: { ...process.env, NO_COLOR: "1", }, - workerData: { - dir, - options, - }, - trackUnmanagedFds: false, }); + worker.unref(); worker.on("message", ({ action, data }) => { if (action === "write-file") { @@ -68,79 +49,67 @@ function runCliWorker(dir, args, options) { } }); + for (const stream of ["stdout", "stderr"]) { + worker[stream].on("data", (data) => { + result[stream] += data.toString(); + }); + } + + const removeStdioFinalNewLine = () => { + for (const stream of ["stdout", "stderr"]) { + result[stream] = removeFinalNewLine(result[stream]); + } + }; + return new Promise((resolve, reject) => { - worker.on("exit", async (code) => { + worker.once("close", (code) => { result.status = code; - result.stdout = removeFinalNewLine(await streamToString(worker.stdout)); - result.stderr = removeFinalNewLine(await streamToString(worker.stderr)); + removeStdioFinalNewLine(); resolve(result); }); - worker.on("error", (error) => { + worker.once("error", (error) => { reject(error); }); - worker.postMessage("run"); + worker.send(options); }); } -async function runPrettierCli(dir, args, options) { +function runPrettierCli(dir, args, options) { dir = path.resolve(INTEGRATION_TEST_DIRECTORY, dir); args = Array.isArray(args) ? args : [args]; - // Worker doesn't support `chdir` - const cwd = process.cwd(); - process.chdir(dir); - - try { - return await runCliWorker(dir, args, options); - } finally { - process.chdir(cwd); - } + return runCliWorker(dir, args, options); } -let runningCli; function runCli(dir, args = [], options = {}) { - let promise; + const promise = runPrettierCli(dir, args, options); const getters = { get status() { - return run().then(({ status }) => status); + return promise.then(({ status }) => status); }, get stdout() { - return run().then(({ stdout }) => stdout); + return promise.then(({ stdout }) => stdout); }, get stderr() { - return run().then(({ stderr }) => stderr); + return promise.then(({ stderr }) => stderr); }, get write() { - return run().then(({ write }) => write); + return promise.then(({ write }) => write); }, test: testResult, then(onFulfilled, onRejected) { - return run().then(onFulfilled, onRejected); + return promise.then(onFulfilled, onRejected); }, }; return getters; - function run() { - if (runningCli) { - throw new Error("Please wait for previous CLI to exit."); - } - - if (!promise) { - promise = runPrettierCli(dir, args, options).finally(() => { - runningCli = undefined; - }); - runningCli = promise; - } - return promise; - } - function testResult(testOptions) { for (const name of ["status", "stdout", "stderr", "write"]) { test(`${options.title || ""}(${name})`, async () => { - const result = await run(); + const result = await promise; let value = result[name]; // \r is trimmed from jest snapshots by default; // manually replacing this character with /*CR*/ to test its true presence