From 8f5218d25f8533f35b877571c9c4041e59939c8f Mon Sep 17 00:00:00 2001 From: Veeti Haapsamo Date: Mon, 8 May 2023 19:35:34 +0300 Subject: [PATCH] fix: add backwards compatibility for old-style run commands (#4195) * fix: add backwards compatibility for old-style run commands * chore: run command error message improvement from code review Co-authored-by: stefreak * test: ensure warnings for legacy-style run command invocations * chore: code review suggestion Co-authored-by: Steffen Neubauer --------- Co-authored-by: stefreak Co-authored-by: Steffen Neubauer --- core/src/commands/run.ts | 80 +++++++--- core/src/commands/workflow.ts | 2 +- .../old-style-run-invocations/garden.yml | 13 ++ .../module-a/garden.yml | 27 ++++ core/test/unit/src/commands/run.ts | 151 ++++++++++-------- 5 files changed, 184 insertions(+), 89 deletions(-) create mode 100644 core/test/data/test-projects/old-style-run-invocations/garden.yml create mode 100644 core/test/data/test-projects/old-style-run-invocations/module-a/garden.yml diff --git a/core/src/commands/run.ts b/core/src/commands/run.ts index af641258c1..d2d3f3f566 100644 --- a/core/src/commands/run.ts +++ b/core/src/commands/run.ts @@ -7,13 +7,24 @@ */ import chalk from "chalk" -import { Command, CommandParams, handleProcessResults, PrepareParams, processCommandResultSchema } from "./base" +import { + Command, + CommandParams, + CommandResult, + handleProcessResults, + PrepareParams, + ProcessCommandResult, + processCommandResultSchema, +} from "./base" import { RunTask } from "../tasks/run" import { printHeader } from "../logger/util" import { ParameterError } from "../exceptions" import { dedent, deline } from "../util/string" import { BooleanParameter, StringsParameter } from "../cli/params" import { validateActionSearchResults, watchParameter, watchRemovedWarning } from "./helpers" +import { Log } from "../logger/log-entry" +import { TestCommand } from "./test" +import { WorkflowCommand, WorkflowRunOutput } from "./workflow" // TODO: support interactive execution for a single Run (needs implementation from RunTask through plugin handlers). @@ -103,7 +114,9 @@ export class RunCommand extends Command { printHeader(headerLog, msg, "🏃‍♂️") } - async action(params: CommandParams) { + async action( + params: CommandParams + ): Promise | CommandResult> { const { garden, log, footerLog, args, opts } = params // Detect possible old-style invocations as early as possible @@ -111,7 +124,11 @@ export class RunCommand extends Command { let names: string[] | undefined = undefined if (args.names && args.names.length > 0) { names = args.names - detectOldRunCommand(names, args, opts) + const result = await maybeOldRunCommand(names, args, opts, log, params) + // If we get a result from the old-style compatibility runner, early return it instead of continuing + if (result) { + return result + } } if (opts.watch) { @@ -195,29 +212,54 @@ export class RunCommand extends Command { } } -function detectOldRunCommand(names: string[], args: any, opts: any) { - if (["module", "service", "task", "test", "workflow"].includes(names[0])) { - let renameDescription = "" - const firstArg = names[0] +/** + * Helper function for detecting an old-style run command invocation and passing the params to the correct handlers. Examples: `garden run workflow foo`, `garden run test`. + * + * This is slightly hacky with any types and other parameter adjusting, but is required for backwards compatibility. + */ +function maybeOldRunCommand(names: string[], args: any, opts: any, log: Log, params: any) { + const firstArg = names[0] + if (["module", "service", "task", "test", "workflow"].includes(firstArg)) { if (firstArg === "module" || firstArg === "service") { - renameDescription = `The ${chalk.white("garden run " + firstArg)} command has been removed. - Please define a Run action instead, or use the underlying tools (e.g. Docker or Kubernetes) directly.` + throw new ParameterError( + `Error: The ${chalk.white("garden run " + firstArg)} command has been removed. + Please define a Run action instead, or use the underlying tools (e.g. Docker or Kubernetes) directly.`, + { args, opts } + ) } if (firstArg === "task") { - renameDescription = `The ${chalk.yellow( - "run task" - )} command was removed in Garden 0.13. Please use the ${chalk.yellow("run")} command instead.` + log.warn( + `The ${chalk.yellow("run task")} command will be removed in Garden 0.14. Please use the ${chalk.yellow( + "run" + )} command instead.` + ) + // Remove the `task` arg and continue execution in the Run handler + names.shift() + return } if (firstArg === "test") { - renameDescription = `The ${chalk.yellow( - "run test" - )} command was removed in Garden 0.13. Please use the ${chalk.yellow("test")} command instead.` + log.warn( + `The ${chalk.yellow("run test")} command will be removed in Garden 0.14. Please use the ${chalk.yellow( + "test" + )} command instead.` + ) + // Remove the `test` arg and execute in the Test handler + names.shift() + const testCmd = new TestCommand() + return testCmd.action(params) } if (firstArg === "workflow") { - renameDescription = `The ${chalk.yellow( - "run workflow" - )} command was removed in Garden 0.13. Please use the ${chalk.yellow("workflow")} command instead.` + log.warn( + `The ${chalk.yellow("run workflow")} command will be removed in Garden 0.14. Please use the ${chalk.yellow( + "workflow" + )} command instead.` + ) + // Remove the `workflow` arg and execute in the Workflow handler + names.shift() + const workflowCmd = new WorkflowCommand() + const workflow = names[0] // NOTE: the workflow command only supports passing one workflow name, not a list + return workflowCmd.action({ ...params, args: { ...args, workflow } }) } - throw new ParameterError(`Error: ${renameDescription}`, { args, opts }) } + return } diff --git a/core/src/commands/workflow.ts b/core/src/commands/workflow.ts index cf4ce70cbe..e6674f65ba 100644 --- a/core/src/commands/workflow.ts +++ b/core/src/commands/workflow.ts @@ -48,7 +48,7 @@ const runWorkflowArgs = { type Args = typeof runWorkflowArgs -interface WorkflowRunOutput { +export interface WorkflowRunOutput { steps: { [stepName: string]: WorkflowStepResult } } diff --git a/core/test/data/test-projects/old-style-run-invocations/garden.yml b/core/test/data/test-projects/old-style-run-invocations/garden.yml new file mode 100644 index 0000000000..71064ea8aa --- /dev/null +++ b/core/test/data/test-projects/old-style-run-invocations/garden.yml @@ -0,0 +1,13 @@ +# TODO: remove this entire test project directory in 0.14 +apiVersion: garden.io/v0 +kind: Project +name: test-project-a +environments: + - name: local +providers: + - name: test-plugin +variables: + some: variable +outputs: + - name: taskName + value: task-a diff --git a/core/test/data/test-projects/old-style-run-invocations/module-a/garden.yml b/core/test/data/test-projects/old-style-run-invocations/module-a/garden.yml new file mode 100644 index 0000000000..e13b540eec --- /dev/null +++ b/core/test/data/test-projects/old-style-run-invocations/module-a/garden.yml @@ -0,0 +1,27 @@ +kind: Module +name: module-a +type: test +variables: + msg: OK +services: + - name: service-a +build: + command: [echo, A] +tests: + - name: unit + command: [echo, "${var.msg}"] + - name: integration + command: [echo, "${var.msg}"] + dependencies: + - service-a +tasks: + - name: task-a + command: [echo, "${var.msg}"] + - name: task-a2 + command: [echo, "${environment.name}-${var.msg}"] + +--- +kind: Workflow +name: workflow-a +steps: + - script: echo diff --git a/core/test/unit/src/commands/run.ts b/core/test/unit/src/commands/run.ts index 9cda84c2a4..83f9427992 100644 --- a/core/test/unit/src/commands/run.ts +++ b/core/test/unit/src/commands/run.ts @@ -8,7 +8,16 @@ import { expect } from "chai" import { RunCommand } from "../../../../src/commands/run" -import { TestGarden, makeTestGardenA, expectError, getAllProcessedTaskNames } from "../../../helpers" +import { + TestGarden, + expectError, + getAllProcessedTaskNames, + makeTestGarden, + getDataDir, + makeTestGardenA, +} from "../../../helpers" +import { getLogMessages } from "../../../../src/util/testing" +import { LogLevel } from "../../../../src/logger/logger" // TODO-G2: fill in test implementations. use TestCommand tests for reference. @@ -89,74 +98,6 @@ describe("RunCommand", () => { ) }) - context("detecting invocations to removed 0.12-era run subcommands", () => { - it("throws if called with 'test' as the first argument", async () => { - await expectError( - () => - garden.runCommand({ - command, - args: { names: ["test", "foo"] }, - opts: { - "force": true, - "force-build": true, - "watch": false, - "skip": [], - "skip-dependencies": false, - "module": undefined, - }, - }), - { - contains: - "The run test command was removed in Garden 0.13", - } - ) - }) - - it("throws if called with 'task' as the first argument", async () => { - await expectError( - () => - garden.runCommand({ - command, - args: { names: ["task", "foo"] }, - opts: { - "force": true, - "force-build": true, - "watch": false, - "skip": [], - "skip-dependencies": false, - "module": undefined, - }, - }), - { - contains: - "The run task command was removed in Garden 0.13", - } - ) - }) - - it("throws if called with 'workflow' as the first argument", async () => { - await expectError( - () => - garden.runCommand({ - command, - args: { names: ["workflow", "foo"] }, - opts: { - "force": true, - "force-build": true, - "watch": false, - "skip": [], - "skip-dependencies": false, - "module": undefined, - }, - }), - { - contains: - "The run workflow command was removed in Garden 0.13", - } - ) - }) - }) - it("supports '*' as an argument to select all Runs", async () => { const { result } = await garden.runCommand({ command, @@ -350,3 +291,75 @@ describe("RunCommand", () => { }) }) }) + +// TODO: switch these back to expectError for 0.14 by reverting a commit from https://github.com/garden-io/garden/pull/4195 +describe("RunCommand legacy invocations", () => { + const command = new RunCommand() + + let garden: TestGarden + + beforeEach(async () => { + garden = await makeTestGarden(getDataDir("test-projects", "old-style-run-invocations")) + }) + + context("detecting invocations to removed 0.12-era run subcommands", async () => { + it("warns if called with 'test' as the first argument", async () => { + const log = garden.log + await garden.runCommand({ + command, + args: { names: ["test", "module-a-unit"] }, + opts: { + "force": true, + "force-build": true, + "watch": false, + "skip": [], + "skip-dependencies": false, + "module": undefined, + }, + }) + const logMessages = getLogMessages(log, (l) => l.level === LogLevel.warn) + expect(logMessages[0]).to.include("The run test command will be removed in Garden 0.14") + }) + + it("warns if called with 'task' as the first argument", async () => { + const log = garden.log + await garden.runCommand({ + command, + args: { names: ["task", "task-a"] }, + opts: { + "force": true, + "force-build": true, + "watch": false, + "skip": [], + "skip-dependencies": false, + "module": undefined, + }, + }) + const logMessages = getLogMessages(log, (l) => l.level === LogLevel.warn) + expect(logMessages[0]).to.include("The run task command will be removed in Garden 0.14") + }) + + it("warns if called with 'workflow' as the first argument", async () => { + const log = garden.log + try { + await garden.runCommand({ + command, + args: { names: ["workflow", "workflow-a"] }, + opts: { + "force": true, + "force-build": true, + "watch": false, + "skip": [], + "skip-dependencies": false, + "module": undefined, + }, + }) + } catch (e) { + // we get an output schema validation error here. + // ignoring it for test purposes - we're only interested in the warning + } + const logMessages = getLogMessages(log, (l) => l.level === LogLevel.warn) + expect(logMessages[0]).to.include("The run workflow command will be removed in Garden 0.14") + }) + }) +})