From 36aeb4ce4f620023c8174e826d7208c0c64f1a0b Mon Sep 17 00:00:00 2001 From: Alexander Kachkaev Date: Wed, 30 Jan 2019 06:08:03 +0000 Subject: [PATCH] Fix CI detection to avoid unwanted TTY behavior (#5804) --- CHANGELOG.unreleased.md | 8 ++++++ package.json | 1 + src/cli/util.js | 7 ++--- src/common/third-party.js | 8 +++--- src/utils/is-tty.js | 10 +++++++ .../__tests__/__snapshots__/check.js.snap | 22 +++++++++++++++ .../__snapshots__/list-different.js.snap | 18 +++++++++++++ tests_integration/__tests__/check.js | 27 +++++++++++++++++++ tests_integration/__tests__/list-different.js | 27 +++++++++++++++++++ tests_integration/runPrettier.js | 6 +++++ yarn.lock | 12 +++++++++ 11 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 src/utils/is-tty.js diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 7b5a8f5be608..3a8dd4dda83f 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -41,3 +41,11 @@ Examples: ``` --> + +- CLI: Fix CI detection to avoid unwanted TTY behavior ([#5804] by [@kachkaev]) + + In Prettier 1.16.0 and 1.16.1, `--list-different` and `--check` logged every file in some CI environments, instead of just unformatted files. + This unwanted behavior is now fixed. + +[#5804]: https://github.com/prettier/prettier/pull/5804 +[@kachkaev]: https://github.com/kachkaev diff --git a/package.json b/package.json index 68799af70865..e86586ddbe3c 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "html-styles": "1.0.0", "html-tag-names": "1.1.2", "ignore": "3.3.7", + "is-ci": "2.0.0", "jest-docblock": "23.2.0", "json-stable-stringify": "1.0.1", "leven": "2.1.0", diff --git a/src/cli/util.js b/src/cli/util.js index 1181c631c78e..bec6b2de7db8 100644 --- a/src/cli/util.js +++ b/src/cli/util.js @@ -19,6 +19,7 @@ const optionsModule = require("../main/options"); const optionsNormalizer = require("../main/options-normalizer"); const thirdParty = require("../common/third-party"); const arrayify = require("../utils/arrayify"); +const isTTY = require("../utils/is-tty"); const OPTION_USAGE_THRESHOLD = 25; const CHOICE_USAGE_MARGIN = 3; @@ -54,7 +55,7 @@ function diff(a, b) { function handleError(context, filename, error) { if (error instanceof errors.UndefinedParserError) { - if (context.argv["write"] && process.stdout.isTTY) { + if (context.argv["write"] && isTTY()) { readline.clearLine(process.stdout, 0); readline.cursorTo(process.stdout, 0, null); } @@ -459,7 +460,7 @@ function formatFiles(context) { return; } - if (process.stdout.isTTY) { + if (isTTY()) { // Don't use `console.log` here since we need to replace this line. context.logger.log(filename, { newline: false }); } @@ -503,7 +504,7 @@ function formatFiles(context) { const isDifferent = output !== input; - if (process.stdout.isTTY) { + if (isTTY()) { // Remove previously printed filename to log it with duration. readline.clearLine(process.stdout, 0); readline.cursorTo(process.stdout, 0, null); diff --git a/src/common/third-party.js b/src/common/third-party.js index e578b1a3efce..5fcdf3c6ca64 100644 --- a/src/common/third-party.js +++ b/src/common/third-party.js @@ -1,11 +1,13 @@ "use strict"; -const getStream = require("get-stream"); const cosmiconfig = require("cosmiconfig"); const findParentDir = require("find-parent-dir").sync; +const getStream = require("get-stream"); +const isCIValue = require("is-ci"); module.exports = { - getStream, cosmiconfig, - findParentDir + findParentDir, + getStream, + isCI: /* istanbul ignore next */ () => isCIValue }; diff --git a/src/utils/is-tty.js b/src/utils/is-tty.js new file mode 100644 index 000000000000..361394ac25b8 --- /dev/null +++ b/src/utils/is-tty.js @@ -0,0 +1,10 @@ +"use strict"; + +const thirdParty = require("../common/third-party"); + +// Some CI pipelines incorrectly report process.stdout.isTTY status, +// which causes unwanted lines in the output. An additional check for isCI() helps. +// See https://github.com/prettier/prettier/issues/5801 +module.exports = function isTTY() { + return process.stdout.isTTY && !thirdParty.isCI(); +}; diff --git a/tests_integration/__tests__/__snapshots__/check.js.snap b/tests_integration/__tests__/__snapshots__/check.js.snap index d91a4aaba463..23d0f8a4131b 100644 --- a/tests_integration/__tests__/__snapshots__/check.js.snap +++ b/tests_integration/__tests__/__snapshots__/check.js.snap @@ -1,5 +1,27 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`--checks works in CI just as in a non-TTY mode (stderr) 1`] = `""`; + +exports[`--checks works in CI just as in a non-TTY mode (stderr) 2`] = `""`; + +exports[`--checks works in CI just as in a non-TTY mode (stdout) 1`] = ` +"Checking formatting... +unformatted.js +Code style issues found in the above file(s). Forgot to run Prettier? +" +`; + +exports[`--checks works in CI just as in a non-TTY mode (stdout) 2`] = ` +"Checking formatting... +unformatted.js +Code style issues found in the above file(s). Forgot to run Prettier? +" +`; + +exports[`--checks works in CI just as in a non-TTY mode (write) 1`] = `Array []`; + +exports[`--checks works in CI just as in a non-TTY mode (write) 2`] = `Array []`; + exports[`checks stdin with --check (write) 1`] = `Array []`; exports[`checks stdin with -c (alias for --check) (write) 1`] = `Array []`; diff --git a/tests_integration/__tests__/__snapshots__/list-different.js.snap b/tests_integration/__tests__/__snapshots__/list-different.js.snap index af19d47b578f..a4de14a0cab9 100644 --- a/tests_integration/__tests__/__snapshots__/list-different.js.snap +++ b/tests_integration/__tests__/__snapshots__/list-different.js.snap @@ -1,5 +1,23 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`--list-different works in CI just as in a non-TTY mode (stderr) 1`] = `""`; + +exports[`--list-different works in CI just as in a non-TTY mode (stderr) 2`] = `""`; + +exports[`--list-different works in CI just as in a non-TTY mode (stdout) 1`] = ` +"unformatted.js +" +`; + +exports[`--list-different works in CI just as in a non-TTY mode (stdout) 2`] = ` +"unformatted.js +" +`; + +exports[`--list-different works in CI just as in a non-TTY mode (write) 1`] = `Array []`; + +exports[`--list-different works in CI just as in a non-TTY mode (write) 2`] = `Array []`; + exports[`checks stdin with --list-different (write) 1`] = `Array []`; exports[`checks stdin with -l (alias for --list-different) (write) 1`] = `Array []`; diff --git a/tests_integration/__tests__/check.js b/tests_integration/__tests__/check.js index 10601eb8c50f..aff86e6ec1b8 100644 --- a/tests_integration/__tests__/check.js +++ b/tests_integration/__tests__/check.js @@ -21,3 +21,30 @@ describe("checks stdin with -c (alias for --check)", () => { status: "non-zero" }); }); + +describe("--checks works in CI just as in a non-TTY mode", () => { + const result0 = runPrettier( + "cli/write", + ["--check", "formatted.js", "unformatted.js"], + { + env: { + CI: "true" + }, + stdoutIsTTY: true + } + ).test({ + status: 1 + }); + + const result1 = runPrettier( + "cli/write", + ["--check", "formatted.js", "unformatted.js"], + { + stdoutIsTTY: false + } + ).test({ + status: 1 + }); + + expect(result0.stdout).toEqual(result1.stdout); +}); diff --git a/tests_integration/__tests__/list-different.js b/tests_integration/__tests__/list-different.js index 93427eea7713..e28342e0abf8 100644 --- a/tests_integration/__tests__/list-different.js +++ b/tests_integration/__tests__/list-different.js @@ -21,3 +21,30 @@ describe("checks stdin with -l (alias for --list-different)", () => { status: "non-zero" }); }); + +describe("--list-different works in CI just as in a non-TTY mode", () => { + const result0 = runPrettier( + "cli/write", + ["--list-different", "formatted.js", "unformatted.js"], + { + env: { + CI: "true" + }, + stdoutIsTTY: true + } + ).test({ + status: 1 + }); + + const result1 = runPrettier( + "cli/write", + ["--list-different", "formatted.js", "unformatted.js"], + { + stdoutIsTTY: false + } + ).test({ + status: 1 + }); + + expect(result0.stdout).toEqual(result1.stdout); +}); diff --git a/tests_integration/runPrettier.js b/tests_integration/runPrettier.js index 99e9a1ede9fd..ae5cabce0829 100644 --- a/tests_integration/runPrettier.js +++ b/tests_integration/runPrettier.js @@ -60,11 +60,13 @@ function runPrettier(dir, args, options) { const originalExitCode = process.exitCode; const originalStdinIsTTY = process.stdin.isTTY; const originalStdoutIsTTY = process.stdout.isTTY; + const originalEnv = process.env; process.chdir(normalizeDir(dir)); process.stdin.isTTY = !!options.isTTY; process.stdout.isTTY = !!options.stdoutIsTTY; process.argv = ["path/to/node", "path/to/prettier/bin"].concat(args); + process.env = Object.assign({}, process.env, options.env); jest.resetModules(); @@ -74,6 +76,9 @@ function runPrettier(dir, args, options) { jest.spyOn(require(thirdParty), "getStream").mockImplementation(() => ({ then: handler => handler(options.input || "") })); + jest + .spyOn(require(thirdParty), "isCI") + .mockImplementation(() => process.env.CI); jest .spyOn(require(thirdParty), "cosmiconfig") .mockImplementation((moduleName, options) => @@ -98,6 +103,7 @@ function runPrettier(dir, args, options) { process.exitCode = originalExitCode; process.stdin.isTTY = originalStdinIsTTY; process.stdout.isTTY = originalStdoutIsTTY; + process.env = originalEnv; jest.restoreAllMocks(); } diff --git a/yarn.lock b/yarn.lock index ff948e5207ea..aecef2bc8f3b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1456,6 +1456,11 @@ ci-info@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/ci-info/-/ci-info-1.0.0.tgz#dc5285f2b4e251821683681c381c3388f46ec534" +ci-info@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/ci-info/-/ci-info-2.0.0.tgz#67a9e964be31a51e15e5010d58e6f12834002f46" + integrity sha512-5tK7EtrZ0N+OLFMthtqOj4fI2Jeb88C4CAZPu25LDVUgXJ0A3Js4PMGqrn0JU1W0Mh1/Z8wZzYPxqUrXeBboCQ== + cipher-base@^1.0.0, cipher-base@^1.0.1, cipher-base@^1.0.3: version "1.0.3" resolved "https://registry.yarnpkg.com/cipher-base/-/cipher-base-1.0.3.tgz#eeabf194419ce900da3018c207d212f2a6df0a07" @@ -3009,6 +3014,13 @@ is-callable@^1.1.1, is-callable@^1.1.3: version "1.1.3" resolved "https://registry.yarnpkg.com/is-callable/-/is-callable-1.1.3.tgz#86eb75392805ddc33af71c92a0eedf74ee7604b2" +is-ci@2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/is-ci/-/is-ci-2.0.0.tgz#6bc6334181810e04b5c22b3d589fdca55026404c" + integrity sha512-YfJT7rkpQB0updsdHLGWrvhBJfcfzNNawYDNIyQXJz0IViGf75O8EBPKSdvw2rF+LGCsX4FZ8tcr3b19LcZq4w== + dependencies: + ci-info "^2.0.0" + is-ci@^1.0.10: version "1.0.10" resolved "https://registry.yarnpkg.com/is-ci/-/is-ci-1.0.10.tgz#f739336b2632365061a9d48270cd56ae3369318e"