From 6ef2cd2b642bcab9086f6d5b956e2d7650ee1db1 Mon Sep 17 00:00:00 2001 From: Gleb Kanterov Date: Wed, 3 Jul 2024 18:46:14 +0200 Subject: [PATCH 1/4] Add tests for Execute --- cmd/root/root.go | 9 ++++++--- main.go | 6 +++++- main_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/cmd/root/root.go b/cmd/root/root.go index 61baa4da0f..87408ff280 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -92,9 +92,10 @@ func flagErrorFunc(c *cobra.Command, err error) error { // Execute adds all child commands to the root command and sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. -func Execute(cmd *cobra.Command) { +// +// Returns exit code for os.Exit. +func Execute(ctx context.Context, cmd *cobra.Command) int { // TODO: deferred panic recovery - ctx := context.Background() // Run the command cmd, err := cmd.ExecuteContextC(ctx) @@ -119,6 +120,8 @@ func Execute(cmd *cobra.Command) { } if err != nil { - os.Exit(1) + return 1 + } else { + return 0 } } diff --git a/main.go b/main.go index 8c8516d9d9..35c11052e9 100644 --- a/main.go +++ b/main.go @@ -2,11 +2,15 @@ package main import ( "context" + "os" "github.com/databricks/cli/cmd" "github.com/databricks/cli/cmd/root" ) func main() { - root.Execute(cmd.New(context.Background())) + ctx := context.Background() + code := root.Execute(ctx, cmd.New(ctx)) + + os.Exit(code) } diff --git a/main_test.go b/main_test.go index 34ecdca0f7..e378f2dfdd 100644 --- a/main_test.go +++ b/main_test.go @@ -1,9 +1,15 @@ package main import ( + "bufio" + "bytes" "context" "testing" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/cmd" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -23,3 +29,45 @@ func TestCommandsDontUseUnderscoreInName(t *testing.T) { queue = append(queue[1:], cmd.Commands()...) } } + +func TestExecute_version(t *testing.T) { + stderr, logger := createFakeLogger() + stdout := bytes.Buffer{} + ctx := cmdio.NewContext(context.Background(), logger) + + cli := cmd.New(ctx) + cli.SetArgs([]string{"version"}) + cli.SetOut(&stdout) + + code := root.Execute(ctx, cli) + + assert.Equal(t, "", stderr.String()) + assert.Contains(t, stdout.String(), "Databricks CLI v") + assert.Equal(t, 0, code) +} + +func TestExecute_unknownCommand(t *testing.T) { + stderr, logger := createFakeLogger() + stdout := bytes.Buffer{} + ctx := cmdio.NewContext(context.Background(), logger) + + cli := cmd.New(ctx) + cli.SetOut(&stdout) + cli.SetArgs([]string{"abcabcabc"}) + + code := root.Execute(ctx, cli) + + assert.Equal(t, `Error: unknown command "abcabcabc" for "databricks"`+"\n", stderr.String()) + assert.Equal(t, "", stdout.String()) + assert.Equal(t, 1, code) +} + +func createFakeLogger() (*bytes.Buffer, *cmdio.Logger) { + out := bytes.Buffer{} + + return &out, &cmdio.Logger{ + Mode: flags.ModeAppend, + Reader: bufio.Reader{}, + Writer: &out, + } +} From b461f95e7784fd655d766afdbe0ca32b66310c1d Mon Sep 17 00:00:00 2001 From: Gleb Kanterov Date: Thu, 4 Jul 2024 09:49:26 +0200 Subject: [PATCH 2/4] Move tests to root_test.go --- cmd/root/root_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++ main_test.go | 48 ------------------------------------- 2 files changed, 55 insertions(+), 48 deletions(-) create mode 100644 cmd/root/root_test.go diff --git a/cmd/root/root_test.go b/cmd/root/root_test.go new file mode 100644 index 0000000000..8192db6759 --- /dev/null +++ b/cmd/root/root_test.go @@ -0,0 +1,55 @@ +package root_test // using 'root' will create circular import +import ( + "bufio" + "bytes" + "context" + "testing" + + "github.com/databricks/cli/cmd" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + "github.com/stretchr/testify/assert" +) + +func TestExecute_version(t *testing.T) { + stderr, logger := createFakeLogger() + stdout := bytes.Buffer{} + ctx := cmdio.NewContext(context.Background(), logger) + + cli := cmd.New(ctx) + cli.SetArgs([]string{"version"}) + cli.SetOut(&stdout) + + code := root.Execute(ctx, cli) + + assert.Equal(t, "", stderr.String()) + assert.Contains(t, stdout.String(), "Databricks CLI v") + assert.Equal(t, 0, code) +} + +func TestExecute_unknownCommand(t *testing.T) { + stderr, logger := createFakeLogger() + stdout := bytes.Buffer{} + ctx := cmdio.NewContext(context.Background(), logger) + + cli := cmd.New(ctx) + cli.SetOut(&stdout) + cli.SetArgs([]string{"abcabcabc"}) + + code := root.Execute(ctx, cli) + + assert.Equal(t, `Error: unknown command "abcabcabc" for "databricks"`+"\n", stderr.String()) + assert.Equal(t, "", stdout.String()) + assert.Equal(t, 1, code) +} + +func createFakeLogger() (*bytes.Buffer, *cmdio.Logger) { + out := bytes.Buffer{} + + return &out, &cmdio.Logger{ + Mode: flags.ModeAppend, + Reader: bufio.Reader{}, + Writer: &out, + } +} diff --git a/main_test.go b/main_test.go index e378f2dfdd..34ecdca0f7 100644 --- a/main_test.go +++ b/main_test.go @@ -1,15 +1,9 @@ package main import ( - "bufio" - "bytes" "context" "testing" - "github.com/databricks/cli/cmd/root" - "github.com/databricks/cli/libs/cmdio" - "github.com/databricks/cli/libs/flags" - "github.com/databricks/cli/cmd" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -29,45 +23,3 @@ func TestCommandsDontUseUnderscoreInName(t *testing.T) { queue = append(queue[1:], cmd.Commands()...) } } - -func TestExecute_version(t *testing.T) { - stderr, logger := createFakeLogger() - stdout := bytes.Buffer{} - ctx := cmdio.NewContext(context.Background(), logger) - - cli := cmd.New(ctx) - cli.SetArgs([]string{"version"}) - cli.SetOut(&stdout) - - code := root.Execute(ctx, cli) - - assert.Equal(t, "", stderr.String()) - assert.Contains(t, stdout.String(), "Databricks CLI v") - assert.Equal(t, 0, code) -} - -func TestExecute_unknownCommand(t *testing.T) { - stderr, logger := createFakeLogger() - stdout := bytes.Buffer{} - ctx := cmdio.NewContext(context.Background(), logger) - - cli := cmd.New(ctx) - cli.SetOut(&stdout) - cli.SetArgs([]string{"abcabcabc"}) - - code := root.Execute(ctx, cli) - - assert.Equal(t, `Error: unknown command "abcabcabc" for "databricks"`+"\n", stderr.String()) - assert.Equal(t, "", stdout.String()) - assert.Equal(t, 1, code) -} - -func createFakeLogger() (*bytes.Buffer, *cmdio.Logger) { - out := bytes.Buffer{} - - return &out, &cmdio.Logger{ - Mode: flags.ModeAppend, - Reader: bufio.Reader{}, - Writer: &out, - } -} From 10462b1f4c3d6a63de28acf920ebc86221012f3d Mon Sep 17 00:00:00 2001 From: Gleb Kanterov Date: Thu, 4 Jul 2024 13:10:57 +0200 Subject: [PATCH 3/4] Add test for unknown command using cobra test runner --- cmd/root/root.go | 10 ++-------- cmd/root/root_test.go | 3 ++- internal/helpers.go | 34 ++++++++++++++++++++++---------- internal/unknown_command_test.go | 15 ++++++++++++++ main.go | 7 ++++--- 5 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 internal/unknown_command_test.go diff --git a/cmd/root/root.go b/cmd/root/root.go index 87408ff280..eda873d122 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -92,9 +92,7 @@ func flagErrorFunc(c *cobra.Command, err error) error { // Execute adds all child commands to the root command and sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. -// -// Returns exit code for os.Exit. -func Execute(ctx context.Context, cmd *cobra.Command) int { +func Execute(ctx context.Context, cmd *cobra.Command) error { // TODO: deferred panic recovery // Run the command @@ -119,9 +117,5 @@ func Execute(ctx context.Context, cmd *cobra.Command) int { } } - if err != nil { - return 1 - } else { - return 0 - } + return err } diff --git a/cmd/root/root_test.go b/cmd/root/root_test.go index 8192db6759..2e5c9b2adf 100644 --- a/cmd/root/root_test.go +++ b/cmd/root/root_test.go @@ -1,4 +1,5 @@ -package root_test // using 'root' will create circular import +package root_test + import ( "bufio" "bytes" diff --git a/internal/helpers.go b/internal/helpers.go index 3923e7e1e2..67a258ba40 100644 --- a/internal/helpers.go +++ b/internal/helpers.go @@ -19,6 +19,9 @@ import ( "testing" "time" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/cmd" _ "github.com/databricks/cli/cmd/version" "github.com/databricks/cli/libs/cmdio" @@ -105,7 +108,12 @@ func (t *cobraTestRunner) registerFlagCleanup(c *cobra.Command) { // Find target command that will be run. Example: if the command run is `databricks fs cp`, // target command corresponds to `cp` targetCmd, _, err := c.Find(t.args) - require.NoError(t, err) + if err != nil && strings.HasPrefix(err.Error(), "unknown command") { + // even if command is unknown, we can proceed + require.NotNil(t, targetCmd) + } else { + require.NoError(t, err) + } // Force initialization of default flags. // These are initialized by cobra at execution time and would otherwise @@ -169,22 +177,28 @@ func (t *cobraTestRunner) RunBackground() { var stdoutW, stderrW io.WriteCloser stdoutR, stdoutW = io.Pipe() stderrR, stderrW = io.Pipe() - root := cmd.New(t.ctx) - root.SetOut(stdoutW) - root.SetErr(stderrW) - root.SetArgs(t.args) + ctx := cmdio.NewContext(t.ctx, &cmdio.Logger{ + Mode: flags.ModeAppend, + Reader: bufio.Reader{}, + Writer: stderrW, + }) + + cli := cmd.New(ctx) + cli.SetOut(stdoutW) + cli.SetErr(stderrW) + cli.SetArgs(t.args) if t.stdinW != nil { - root.SetIn(t.stdinR) + cli.SetIn(t.stdinR) } // Register cleanup function to restore flags to their original values // once test has been executed. This is needed because flag values reside // in a global singleton data-structure, and thus subsequent tests might // otherwise interfere with each other - t.registerFlagCleanup(root) + t.registerFlagCleanup(cli) errch := make(chan error) - ctx, cancel := context.WithCancel(t.ctx) + ctx, cancel := context.WithCancel(ctx) // Tee stdout/stderr to buffers. stdoutR = io.TeeReader(stdoutR, &t.stdout) @@ -197,7 +211,7 @@ func (t *cobraTestRunner) RunBackground() { // Run command in background. go func() { - cmd, err := root.ExecuteContextC(ctx) + err := root.Execute(ctx, cli) if err != nil { t.Logf("Error running command: %s", err) } @@ -230,7 +244,7 @@ func (t *cobraTestRunner) RunBackground() { // These commands are globals so we have to clean up to the best of our ability after each run. // See https://github.com/spf13/cobra/blob/a6f198b635c4b18fff81930c40d464904e55b161/command.go#L1062-L1066 //lint:ignore SA1012 cobra sets the context and doesn't clear it - cmd.SetContext(nil) + cli.SetContext(nil) // Make caller aware of error. errch <- err diff --git a/internal/unknown_command_test.go b/internal/unknown_command_test.go new file mode 100644 index 0000000000..62b84027f1 --- /dev/null +++ b/internal/unknown_command_test.go @@ -0,0 +1,15 @@ +package internal + +import ( + "testing" + + assert "github.com/databricks/cli/libs/dyn/dynassert" +) + +func TestUnknownCommand(t *testing.T) { + stdout, stderr, err := RequireErrorRun(t, "unknown-command") + + assert.Error(t, err, "unknown command", `unknown command "unknown-command" for "databricks"`) + assert.Equal(t, "", stdout.String()) + assert.Contains(t, stderr.String(), "unknown command") +} diff --git a/main.go b/main.go index 35c11052e9..c568e6adbd 100644 --- a/main.go +++ b/main.go @@ -10,7 +10,8 @@ import ( func main() { ctx := context.Background() - code := root.Execute(ctx, cmd.New(ctx)) - - os.Exit(code) + err := root.Execute(ctx, cmd.New(ctx)) + if err != nil { + os.Exit(1) + } } From 5a312abba147d02aac20808850561c8caad3b03b Mon Sep 17 00:00:00 2001 From: Gleb Kanterov Date: Thu, 4 Jul 2024 13:15:54 +0200 Subject: [PATCH 4/4] Remove root_test.go --- cmd/root/root_test.go | 56 ------------------------------------------- 1 file changed, 56 deletions(-) delete mode 100644 cmd/root/root_test.go diff --git a/cmd/root/root_test.go b/cmd/root/root_test.go deleted file mode 100644 index 2e5c9b2adf..0000000000 --- a/cmd/root/root_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package root_test - -import ( - "bufio" - "bytes" - "context" - "testing" - - "github.com/databricks/cli/cmd" - "github.com/databricks/cli/cmd/root" - "github.com/databricks/cli/libs/cmdio" - "github.com/databricks/cli/libs/flags" - "github.com/stretchr/testify/assert" -) - -func TestExecute_version(t *testing.T) { - stderr, logger := createFakeLogger() - stdout := bytes.Buffer{} - ctx := cmdio.NewContext(context.Background(), logger) - - cli := cmd.New(ctx) - cli.SetArgs([]string{"version"}) - cli.SetOut(&stdout) - - code := root.Execute(ctx, cli) - - assert.Equal(t, "", stderr.String()) - assert.Contains(t, stdout.String(), "Databricks CLI v") - assert.Equal(t, 0, code) -} - -func TestExecute_unknownCommand(t *testing.T) { - stderr, logger := createFakeLogger() - stdout := bytes.Buffer{} - ctx := cmdio.NewContext(context.Background(), logger) - - cli := cmd.New(ctx) - cli.SetOut(&stdout) - cli.SetArgs([]string{"abcabcabc"}) - - code := root.Execute(ctx, cli) - - assert.Equal(t, `Error: unknown command "abcabcabc" for "databricks"`+"\n", stderr.String()) - assert.Equal(t, "", stdout.String()) - assert.Equal(t, 1, code) -} - -func createFakeLogger() (*bytes.Buffer, *cmdio.Logger) { - out := bytes.Buffer{} - - return &out, &cmdio.Logger{ - Mode: flags.ModeAppend, - Reader: bufio.Reader{}, - Writer: &out, - } -}