fix: ensure CLI returns correct exit code (#626)

...also changed LoggerType from enum to union type

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
       }
 

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(),
 ]

garden-service/src/commands/exec.ts

@@ -61,7 +61,7 @@ export class ExecCommand extends Command<Args> {
 
   arguments = runArgs
   options = runOpts
-  loggerType = LoggerType.basic
+  loggerType: LoggerType = "basic"
 
   async action({ garden, log, args, opts }: CommandParams<Args, Opts>): Promise<CommandResult<ExecInServiceResult>> {
     const serviceName = args.service

garden-service/src/commands/logs.ts

@@ -64,7 +64,7 @@ export class LogsCommand extends Command<Args, Opts> {
 
   arguments = logsArgs
   options = logsOpts
-  loggerType = LoggerType.basic
+  loggerType: LoggerType = "basic"
 
   async action({ garden, log, args, opts }: CommandParams<Args, Opts>): Promise<CommandResult<ServiceLogEntry[]>> {
     const { follow, tail } = opts

garden-service/src/commands/serve.ts

@@ -30,7 +30,7 @@ export class ServeCommand extends Command<Args, Opts> {
   help = "Starts the Garden HTTP API service - **Experimental**"
 
   cliOnly = true
-  loggerType = LoggerType.basic
+  loggerType: LoggerType = "basic"
 
   description = dedent`
     **Experimental**

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<LoggerType>(["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 = <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,
         })
       }
 

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