From eeb069f9a27518d9eac035f018a2c249ed90048b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Tue, 26 Mar 2019 15:51:10 +0100 Subject: [PATCH] fix: ensure CLI returns correct exit code (#626) ...also changed LoggerType from enum to union type --- garden-service/src/cli/cli.ts | 43 +++++++++++-------- garden-service/src/commands/commands.ts | 2 - garden-service/src/commands/exec.ts | 2 +- garden-service/src/commands/logs.ts | 2 +- garden-service/src/commands/serve.ts | 2 +- garden-service/src/commands/version.ts | 27 ------------ garden-service/src/logger/logger.ts | 24 +++++------ garden-service/test/integ/run | 36 ++++++++++++++++ .../test/unit/src/commands/version.ts | 19 -------- 9 files changed, 73 insertions(+), 84 deletions(-) delete mode 100644 garden-service/src/commands/version.ts delete mode 100644 garden-service/test/unit/src/commands/version.ts diff --git a/garden-service/src/cli/cli.ts b/garden-service/src/cli/cli.ts index d95634c0ff..073e331784 100644 --- a/garden-service/src/cli/cli.ts +++ b/garden-service/src/cli/cli.ts @@ -25,7 +25,7 @@ import { } from "../commands/base" import { GardenError, PluginError, toGardenError } from "../exceptions" import { Garden, GardenOpts } from "../garden" -import { getLogger, Logger, LoggerType } from "../logger/logger" +import { getLogger, Logger, LoggerType, LOGGER_TYPES } from "../logger/logger" import { LogLevel } from "../logger/log-node" import { BasicTerminalWriter } from "../logger/writers/basic-terminal-writer" import { FancyTerminalWriter } from "../logger/writers/fancy-terminal-writer" @@ -62,9 +62,9 @@ const OUTPUT_RENDERERS = { } const WRITER_CLASSES = { - [LoggerType.basic]: BasicTerminalWriter, - [LoggerType.fancy]: FancyTerminalWriter, - [LoggerType.json]: JsonTerminalWriter, + basic: BasicTerminalWriter, + fancy: FancyTerminalWriter, + json: JsonTerminalWriter, } const FILE_WRITER_CONFIGS = [ @@ -74,7 +74,7 @@ const FILE_WRITER_CONFIGS = [ ] const GLOBAL_OPTIONS_GROUP_NAME = "Global options" -const DEFAULT_CLI_LOGGER_TYPE = LoggerType.fancy +const DEFAULT_CLI_LOGGER_TYPE = "fancy" // For initializing garden without a project config export const MOCK_CONFIG: GardenConfig = { @@ -97,22 +97,29 @@ export const MOCK_CONFIG: GardenConfig = { } export const GLOBAL_OPTIONS = { - root: new StringParameter({ + "root": new StringParameter({ alias: "r", help: "Override project root directory (defaults to working directory).", defaultValue: process.cwd(), }), - silent: new BooleanParameter({ + "silent": new BooleanParameter({ alias: "s", help: "Suppress log output.", defaultValue: false, }), - env: new EnvironmentOption(), - loggerType: new ChoicesParameter({ - choices: Object.keys(WRITER_CLASSES), - help: `TODO`, + "env": new EnvironmentOption(), + "logger-type": new ChoicesParameter({ + choices: [...LOGGER_TYPES], + defaultValue: DEFAULT_CLI_LOGGER_TYPE, + help: deline` + Set logger type: + fancy: updates log lines in-place when their status changes (e.g. when tasks complete), + basic: appends a new log line when a log line's status changes, + json: same as basic, but renders log lines as JSON, + quiet: uppresses all log output, + `, }), - loglevel: new ChoicesParameter({ + "loglevel": new ChoicesParameter({ alias: "l", choices: getLogLevelChoices(), help: deline` @@ -122,12 +129,12 @@ export const GLOBAL_OPTIONS = { "[enum] [default: info] [error || 0, warn || 1, info || 2, verbose || 3, debug || 4, silly || 5]", defaultValue: LogLevel[LogLevel.info], }), - output: new ChoicesParameter({ + "output": new ChoicesParameter({ alias: "o", choices: Object.keys(OUTPUT_RENDERERS), help: "Output command result in specified format (note: disables progress logging and interactive functionality).", }), - emoji: new BooleanParameter({ + "emoji": new BooleanParameter({ help: "Enable emoji in output (defaults to true if the environment supports it).", defaultValue: envSupportsEmoji(), }), @@ -166,13 +173,11 @@ export class GardenCli { this.program = sywac .help("-h, --help", { group: GLOBAL_OPTIONS_GROUP_NAME, - implicitCommand: false, }) .version("-v, --version", { version, group: GLOBAL_OPTIONS_GROUP_NAME, description: "Show's the current cli version.", - implicitCommand: false, }) .showHelpByDefault() .check((argv, _ctx) => { @@ -240,12 +245,12 @@ export class GardenCli { const parsedArgs = filterByKeys(argv, argKeys) const parsedOpts = filterByKeys(argv, optKeys.concat(globalKeys)) const root = resolve(process.cwd(), parsedOpts.root) - const { emoji, env, loglevel, loggerType: loggerTypeOpt, silent, output } = parsedOpts + const { emoji, env, loglevel, "logger-type": loggerTypeOpt, silent, output } = parsedOpts const loggerType = loggerTypeOpt || command.loggerType || DEFAULT_CLI_LOGGER_TYPE // Init logger - const logEnabled = !silent && !output && loggerType !== LoggerType.quiet + const logEnabled = !silent && !output && loggerType !== "quiet" const level = parseLogLevel(loglevel) const logger = initLogger({ level, logEnabled, loggerType, emoji }) @@ -347,7 +352,7 @@ export class GardenCli { console.log(cliOutput) // fix issue where sywac returns exit code 0 even when a command doesn't exist - if (!argv.h && !argv.help) { + if (!argv.h && !argv.v) { code = 1 } diff --git a/garden-service/src/commands/commands.ts b/garden-service/src/commands/commands.ts index 3e3a9592e3..062e22bae7 100644 --- a/garden-service/src/commands/commands.ts +++ b/garden-service/src/commands/commands.ts @@ -26,7 +26,6 @@ import { UpdateRemoteCommand } from "./update-remote/update-remote" import { ValidateCommand } from "./validate" import { ExecCommand } from "./exec" import { ServeCommand } from "./serve" -import { VersionCommand } from "./version" export const coreCommands: Command[] = [ new BuildCommand(), @@ -48,5 +47,4 @@ export const coreCommands: Command[] = [ new UnlinkCommand(), new UpdateRemoteCommand(), new ValidateCommand(), - new VersionCommand(), ] diff --git a/garden-service/src/commands/exec.ts b/garden-service/src/commands/exec.ts index 2eb636fb60..59186d050a 100644 --- a/garden-service/src/commands/exec.ts +++ b/garden-service/src/commands/exec.ts @@ -61,7 +61,7 @@ export class ExecCommand extends Command { arguments = runArgs options = runOpts - loggerType = LoggerType.basic + loggerType: LoggerType = "basic" async action({ garden, log, args, opts }: CommandParams): Promise> { const serviceName = args.service diff --git a/garden-service/src/commands/logs.ts b/garden-service/src/commands/logs.ts index 3107cb70e3..344b552af9 100644 --- a/garden-service/src/commands/logs.ts +++ b/garden-service/src/commands/logs.ts @@ -64,7 +64,7 @@ export class LogsCommand extends Command { arguments = logsArgs options = logsOpts - loggerType = LoggerType.basic + loggerType: LoggerType = "basic" async action({ garden, log, args, opts }: CommandParams): Promise> { const { follow, tail } = opts diff --git a/garden-service/src/commands/serve.ts b/garden-service/src/commands/serve.ts index c0d3ccf6f4..43735df493 100644 --- a/garden-service/src/commands/serve.ts +++ b/garden-service/src/commands/serve.ts @@ -30,7 +30,7 @@ export class ServeCommand extends Command { help = "Starts the Garden HTTP API service - **Experimental**" cliOnly = true - loggerType = LoggerType.basic + loggerType: LoggerType = "basic" description = dedent` **Experimental** diff --git a/garden-service/src/commands/version.ts b/garden-service/src/commands/version.ts deleted file mode 100644 index 82313e92b0..0000000000 --- a/garden-service/src/commands/version.ts +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright (C) 2018 Garden Technologies, Inc. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - */ - -import { - Command, - CommandResult, - CommandParams, -} from "./base" -import chalk from "chalk" -import { getPackageVersion } from "../util/util" - -export class VersionCommand extends Command { - name = "version" - help = "Show's the current cli version." - - async action({ log }: CommandParams<{}>): Promise> { - const result = `${getPackageVersion()}` - - log.info(`${chalk.white(result)}`) - return { result } - } -} diff --git a/garden-service/src/logger/logger.ts b/garden-service/src/logger/logger.ts index 3ff72eab55..b099451b29 100644 --- a/garden-service/src/logger/logger.ts +++ b/garden-service/src/logger/logger.ts @@ -17,27 +17,23 @@ import { FancyTerminalWriter } from "./writers/fancy-terminal-writer" import { JsonTerminalWriter } from "./writers/json-terminal-writer" import { parseLogLevel } from "../cli/helpers" -export enum LoggerType { - quiet = "quiet", - basic = "basic", - fancy = "fancy", - json = "json", -} +export type LoggerType = "quiet" | "basic" | "fancy" | "json" +export const LOGGER_TYPES = new Set(["quiet", "basic", "fancy", "json"]) export function getCommonConfig(loggerType: LoggerType): LoggerConfig { const configs: { [key in LoggerType]: LoggerConfig } = { - [LoggerType.quiet]: { + quiet: { level: LogLevel.info, }, - [LoggerType.basic]: { + basic: { level: LogLevel.info, writers: [new BasicTerminalWriter()], }, - [LoggerType.fancy]: { + fancy: { level: LogLevel.info, writers: [new FancyTerminalWriter()], }, - [LoggerType.json]: { + json: { level: LogLevel.info, writers: [new JsonTerminalWriter()], }, @@ -84,12 +80,12 @@ export class Logger extends LogNode { // GARDEN_LOGGER_TYPE env variable takes precedence over the config param if (process.env.GARDEN_LOGGER_TYPE) { - const loggerType = LoggerType[process.env.GARDEN_LOGGER_TYPE] + const loggerType = process.env.GARDEN_LOGGER_TYPE - if (!loggerType) { - throw new ParameterError(`Invalid logger type specified: ${process.env.GARDEN_LOGGER_TYPE}`, { + if (!LOGGER_TYPES.has(loggerType)) { + throw new ParameterError(`Invalid logger type specified: ${loggerType}`, { loggerType: process.env.GARDEN_LOGGER_TYPE, - availableTypes: Object.keys(LoggerType), + availableTypes: LOGGER_TYPES, }) } diff --git a/garden-service/test/integ/run b/garden-service/test/integ/run index 65dffda2d0..2a83396426 100755 --- a/garden-service/test/integ/run +++ b/garden-service/test/integ/run @@ -18,6 +18,42 @@ if [[ $? -eq 0 ]]; then exit 1 fi +${garden_bin} version +if [[ $? -ne 0 ]]; then + echo "Expected exit code 0 when calling version" + exit 1 +fi + +${garden_bin} --version +if [[ $? -ne 0 ]]; then + echo "Expected exit code 0 when calling --version" + exit 1 +fi + +${garden_bin} -v +if [[ $? -ne 0 ]]; then + echo "Expected exit code 0 when calling -v" + exit 1 +fi + +${garden_bin} help +if [[ $? -ne 0 ]]; then + echo "Expected exit code 0 when calling help" + exit 1 +fi + +${garden_bin} --help +if [[ $? -ne 0 ]]; then + echo "Expected exit code 0 when calling --help" + exit 1 +fi + +${garden_bin} -h +if [[ $? -ne 0 ]]; then + echo "Expected exit code 0 when calling --help" + exit 1 +fi + set -e ${garden_bin} scan diff --git a/garden-service/test/unit/src/commands/version.ts b/garden-service/test/unit/src/commands/version.ts deleted file mode 100644 index 62bac66efd..0000000000 --- a/garden-service/test/unit/src/commands/version.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { expect } from "chai" -import { VersionCommand } from "../../../../src/commands/version" -import { makeTestGardenA } from "../../../helpers" - -describe("VersionCommand", () => { - it("should return the current package's version", async () => { - const command = new VersionCommand() - const garden = await makeTestGardenA() - const log = garden.log - const result = await command.action({ - log, - garden, - args: {}, - opts: {}, - }) - - expect(result.result).to.eql(require("../../../../package.json").version) - }) -})