From a7fec56e830c6ec71abc04a28d8d21fce533c389 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 27 Feb 2019 12:12:17 +0200 Subject: [PATCH] fix: errors from `tns doctor` are not visible in CI environment When `tns doctor` detects errors in the configuration, it should print them to stdout and prompt the user to select how to resolve them. In non-interactive terminal, it should just print the errors and fails as the prompter are not meaningful in non-interactive terminal - there's noone to answer them. The logic works fine when you pipe the output of `tns doctor` to file for example (`tns doctor > out.txt`). CLI correctly detects the terminal as non-interactive, prints the errors and exits. However, most of the CI environments are determined as interactive terminals by CLI. That's because CLI checks if the stdout and stdin of the current process are text terminal (TTY). CI environments set required flags, so the process seems like running in such text terminal. When CLI thinks the process is running in text terminal, it uses some external package (ora) to print pretty lines. However, `ora` package also checks if the process is running in text terminal (which both CLI and `ora` think is true), but it also checks if the environment variable `CI` is set. When it is set, `ora` package decides that it cannot print colored messages and just doesn't print anything. To resolve this, improve the check if interactive terminal in CLI to respect the known environment variables that define the process as running in CI: - for Travis the environment varible `CI` is set. - for CircleCI the environment variable `CI` is set - for Jenkins the environment variable `JENKINS_HOME` is set Whenever one of those environment variables is set, CLI will decide it is running in non-interactive terminal and will not use `ora` (for `tns doctor`). It will also not show any prompters in this case. --- lib/common/helpers.ts | 22 +++++++- lib/common/test/unit-tests/helpers.ts | 51 +++++++++++++++++++ test/services/doctor-service.ts | 11 ---- .../platform-environment-requirements.ts | 13 ++++- 4 files changed, 83 insertions(+), 14 deletions(-) diff --git a/lib/common/helpers.ts b/lib/common/helpers.ts index 29089ee08f..784848eb26 100644 --- a/lib/common/helpers.ts +++ b/lib/common/helpers.ts @@ -309,7 +309,27 @@ export function versionCompare(version1: string | IVersionData, version2: string } export function isInteractive(): boolean { - return process.stdout.isTTY && process.stdin.isTTY; + const isInteractive = isRunningInTTY() && !isCIEnvironment(); + return isInteractive; +} + +/** + * Checks if current process is running in Text Terminal (TTY) + */ +function isRunningInTTY(): boolean { + return process.stdout && + process.stdout.isTTY && + process.stdin && + process.stdin.isTTY; +} + +function isCIEnvironment(): boolean { + // The following CI environments set their own environment variables that we respect: + // travis: "CI", + // circleCI: "CI", + // jenkins: "JENKINS_HOME" + + return !!(process.env && (process.env.CI || process.env.JENKINS_HOME)); } export function toBoolean(str: any): boolean { diff --git a/lib/common/test/unit-tests/helpers.ts b/lib/common/test/unit-tests/helpers.ts index f21c1db2ec..5edc0dd7d9 100644 --- a/lib/common/test/unit-tests/helpers.ts +++ b/lib/common/test/unit-tests/helpers.ts @@ -861,4 +861,55 @@ const test = require("./test");`, }); }); }); + + describe("isInteractive", () => { + const originalEnv = process.env; + const originalStdoutIsTTY = process.stdout.isTTY; + const originalStdinIsTTY = process.stdin.isTTY; + beforeEach(() => { + process.env.CI = ""; + process.env.JENKINS_HOME = ""; + }); + + afterEach(() => { + process.env = originalEnv; + process.stdout.isTTY = originalStdoutIsTTY; + process.stdin.isTTY = originalStdinIsTTY; + }); + + it("returns false when stdout is not TTY", () => { + (process.stdout).isTTY = false; + (process.stdin).isTTY = true; + assert.isFalse(helpers.isInteractive()); + }); + + it("returns false when stdin is not TTY", () => { + (process.stdin).isTTY = false; + (process.stdout).isTTY = true; + assert.isFalse(helpers.isInteractive()); + }); + + it("returns false when stdout and stdin are TTY, but CI env var is set", () => { + (process.stdout).isTTY = true; + (process.stdin).isTTY = true; + process.env.CI = "true"; + + assert.isFalse(helpers.isInteractive()); + }); + + it("returns false when stdout and stdin are TTY, but JENKINS_HOME env var is set", () => { + (process.stdout).isTTY = true; + (process.stdin).isTTY = true; + process.env.JENKINS_HOME = "/usr/local/lib/jenkins"; + + assert.isFalse(helpers.isInteractive()); + }); + + it("returns true when stdout and stdin are TTY and neither CI or JENKINS_HOME are set", () => { + (process.stdout).isTTY = true; + (process.stdin).isTTY = true; + + assert.isTrue(helpers.isInteractive()); + }); + }); }); diff --git a/test/services/doctor-service.ts b/test/services/doctor-service.ts index 31617b009a..0b55e96b9f 100644 --- a/test/services/doctor-service.ts +++ b/test/services/doctor-service.ts @@ -111,17 +111,6 @@ describe("doctorService", () => { filesContents: { file1: `const application = require("application"); const Observable = require("data/observable").Observable; -` - }, - expectedShortImports: [ - { file: "file1", line: 'const application = require("application");' }, - { file: "file1", line: 'const Observable = require("data/observable").Observable;' }, - ] - }, - { - filesContents: { - file1: `const application = require("application"); -const Observable = require("data/observable").Observable; ` }, expectedShortImports: [ diff --git a/test/services/platform-environment-requirements.ts b/test/services/platform-environment-requirements.ts index 5dab5825ff..9699a2c91e 100644 --- a/test/services/platform-environment-requirements.ts +++ b/test/services/platform-environment-requirements.ts @@ -3,7 +3,9 @@ import { PlatformEnvironmentRequirements } from '../../lib/services/platform-env import * as stubs from "../stubs"; import { assert } from "chai"; import { EOL } from "os"; +const helpers = require("../../lib/common/helpers"); +const originalIsInteractive = helpers.isInteractive; const platform = "android"; const cloudBuildsErrorMessage = `In order to test your application use the $ tns login command to log in with your account and then $ tns cloud build command to build your app in the cloud.`; const manuallySetupErrorMessage = `To be able to build for ${platform}, verify that your environment is configured according to the system requirements described at `; @@ -34,6 +36,14 @@ function createTestInjector() { } describe("platformEnvironmentRequirements ", () => { + beforeEach(() => { + helpers.isInteractive = () => true; + }); + + afterEach(() => { + helpers.isInteractive = originalIsInteractive; + }); + describe("checkRequirements", () => { let testInjector: IInjector = null; let platformEnvironmentRequirements: IPlatformEnvironmentRequirements = null; @@ -221,8 +231,7 @@ describe("platformEnvironmentRequirements ", () => { describe("when console is non interactive", () => { beforeEach(() => { - (process).stdout.isTTY = false; - (process.stdin).isTTY = false; + helpers.isInteractive = () => false; mockDoctorService({ canExecuteLocalBuild: false }); });