From 758dceb59cf097c80addbab215d38d0d3cce2ad9 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 16:11:28 +0000 Subject: [PATCH 1/8] feat(cli): add test for delete This adds a new test for the `delete` command to ensure it works as expected when provided the correct args. --- cli/delete_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 cli/delete_test.go diff --git a/cli/delete_test.go b/cli/delete_test.go new file mode 100644 index 0000000000000..2f67fc52bd38f --- /dev/null +++ b/cli/delete_test.go @@ -0,0 +1,38 @@ +package cli_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/cli/clitest" + "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/pty/ptytest" +) + +func TestDelete(t *testing.T) { + t.Run("WithParameter", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + coderdtest.NewProvisionerDaemon(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + cmd, root := clitest.New(t, "delete", workspace.Name) + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + go func() { + defer close(doneChan) + err := cmd.Execute() + require.NoError(t, err) + }() + pty.ExpectMatch("Cleaning Up") + <-doneChan + }) +} From 6ff347ec4cd57fc41f2a4df6bcf4b0c613144c0e Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 16:23:33 +0000 Subject: [PATCH 2/8] fix(cli): use ExecuteC() to match Cobra This modifies the `cli.Root().Execute()` to `cli.Root).ExecuteC()` to match the default behavior of Cobra. We do this so errors will always print the "run --help" line. --- cmd/coder/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/coder/main.go b/cmd/coder/main.go index 1c09c6985185b..1c7c2f7982e56 100644 --- a/cmd/coder/main.go +++ b/cmd/coder/main.go @@ -14,7 +14,7 @@ import ( func main() { dadjoke() - err := cli.Root().Execute() + _, err := cli.Root().ExecuteC() if err != nil { if errors.Is(err, cliui.Canceled) { os.Exit(1) From 16c19c7c4ed3894ae9cf12f1a78ac7a537fbe2aa Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 16:26:30 +0000 Subject: [PATCH 3/8] feat(cli): add WithoutParameters test for delete This adds a new test to the `delete_test.go` suite to ensure the correct behavior occurs when `delete` is called without an argument. --- cli/delete_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cli/delete_test.go b/cli/delete_test.go index 2f67fc52bd38f..dd3312caa98fa 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -35,4 +35,12 @@ func TestDelete(t *testing.T) { pty.ExpectMatch("Cleaning Up") <-doneChan }) + t.Run("WithoutParameters", func(t *testing.T) { + t.Parallel() + + cmd, _ := clitest.New(t, "delete") + + err := cmd.Execute() + require.ErrorContains(t, err, "Run 'coder workspaces delete --help' for usage.") + }) } From dae45abaa8fa770285c5b3f2326260c16057e0f4 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 18:25:10 +0000 Subject: [PATCH 4/8] fixup! feat(cli): add WithoutParameters test for delete --- cli/delete_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/delete_test.go b/cli/delete_test.go index dd3312caa98fa..9a447bce4db93 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -41,6 +41,6 @@ func TestDelete(t *testing.T) { cmd, _ := clitest.New(t, "delete") err := cmd.Execute() - require.ErrorContains(t, err, "Run 'coder workspaces delete --help' for usage.") + require.ErrorContains(t, err, "Run 'coder delete --help' for usage.") }) } From 32cbc855f8cf99697f5f868956db3ea49a87f43c Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 18:25:27 +0000 Subject: [PATCH 5/8] refactor(cli): show --help error message on main This adds an error message which shows when there is an error with any commands called to improve the UX. --- cmd/coder/main.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/coder/main.go b/cmd/coder/main.go index 1c7c2f7982e56..44c17d38ea74f 100644 --- a/cmd/coder/main.go +++ b/cmd/coder/main.go @@ -14,12 +14,13 @@ import ( func main() { dadjoke() - _, err := cli.Root().ExecuteC() + cmd, err := cli.Root().ExecuteC() if err != nil { if errors.Is(err, cliui.Canceled) { os.Exit(1) } - _, _ = fmt.Fprintln(os.Stderr, cliui.Styles.Error.Render(err.Error())) + helpErrMsg := fmt.Sprintf("Run '%s %s --help' for usage.", cmd.Root().Name(), cmd.Name()) + _, _ = fmt.Fprintln(os.Stderr, cliui.Styles.Error.Render(err.Error()+"/n"+helpErrMsg)) os.Exit(1) } } From a7bd7bd7b6372b03068cc7cdaec11ee5550a71ba Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 19:09:58 +0000 Subject: [PATCH 6/8] fixup! refactor(cli): show --help error message on main --- cli/delete_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cli/delete_test.go b/cli/delete_test.go index 9a447bce4db93..6b624ebcf5b45 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/coder/coder/cli" "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/pty/ptytest" @@ -35,12 +36,13 @@ func TestDelete(t *testing.T) { pty.ExpectMatch("Cleaning Up") <-doneChan }) - t.Run("WithoutParameters", func(t *testing.T) { + t.Run("WithoutParameters/FormatCobraError", func(t *testing.T) { t.Parallel() cmd, _ := clitest.New(t, "delete") - err := cmd.Execute() - require.ErrorContains(t, err, "Run 'coder delete --help' for usage.") + cmd, err := cmd.ExecuteC() + errStr := cli.FormatCobraError(err, cmd) + require.Contains(t, errStr, "Run 'coder delete --help' for usage.") }) } From c28e93a7890da08ef8393a6d66eb60de2fe6e131 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 19:10:08 +0000 Subject: [PATCH 7/8] refactor(cli): handle err with FormatCobraError This adds a new helper function called `FormatCobraError` to `root.go` so that we can colorize and add "--help" message to cobra command errors like calling `delete`. --- cli/root.go | 7 +++++++ cmd/coder/main.go | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cli/root.go b/cli/root.go index 424ec54155c03..416db617a9d4c 100644 --- a/cli/root.go +++ b/cli/root.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "net/url" "os" "time" @@ -259,3 +260,9 @@ func versionTemplate() string { template += "\r\n" return template } + +// FormatCobraError colorizes and adds "--help" docs to cobra commands. +func FormatCobraError(err error, cmd *cobra.Command) string { + helpErrMsg := fmt.Sprintf("Run '%s %s --help' for usage.", cmd.Root().Name(), cmd.Name()) + return cliui.Styles.Error.Render(err.Error() + "\n" + helpErrMsg) +} diff --git a/cmd/coder/main.go b/cmd/coder/main.go index 44c17d38ea74f..f6c2b511ab2d0 100644 --- a/cmd/coder/main.go +++ b/cmd/coder/main.go @@ -19,8 +19,8 @@ func main() { if errors.Is(err, cliui.Canceled) { os.Exit(1) } - helpErrMsg := fmt.Sprintf("Run '%s %s --help' for usage.", cmd.Root().Name(), cmd.Name()) - _, _ = fmt.Fprintln(os.Stderr, cliui.Styles.Error.Render(err.Error()+"/n"+helpErrMsg)) + cobraErr := cli.FormatCobraError(err, cmd) + _, _ = fmt.Fprintln(os.Stderr, cobraErr) os.Exit(1) } } From 8ac2198012d2c179fe764fabb2f6e0a3e40f5945 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 19 May 2022 22:24:21 +0000 Subject: [PATCH 8/8] refactor(cli): add root_test.go, move delete test --- cli/delete_test.go | 10 ---------- cli/root_test.go | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 cli/root_test.go diff --git a/cli/delete_test.go b/cli/delete_test.go index 6b624ebcf5b45..2f67fc52bd38f 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -5,7 +5,6 @@ import ( "github.com/stretchr/testify/require" - "github.com/coder/coder/cli" "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/pty/ptytest" @@ -36,13 +35,4 @@ func TestDelete(t *testing.T) { pty.ExpectMatch("Cleaning Up") <-doneChan }) - t.Run("WithoutParameters/FormatCobraError", func(t *testing.T) { - t.Parallel() - - cmd, _ := clitest.New(t, "delete") - - cmd, err := cmd.ExecuteC() - errStr := cli.FormatCobraError(err, cmd) - require.Contains(t, errStr, "Run 'coder delete --help' for usage.") - }) } diff --git a/cli/root_test.go b/cli/root_test.go new file mode 100644 index 0000000000000..28206cc46707f --- /dev/null +++ b/cli/root_test.go @@ -0,0 +1,22 @@ +package cli_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/cli" + "github.com/coder/coder/cli/clitest" +) + +func TestRoot(t *testing.T) { + t.Run("FormatCobraError", func(t *testing.T) { + t.Parallel() + + cmd, _ := clitest.New(t, "delete") + + cmd, err := cmd.ExecuteC() + errStr := cli.FormatCobraError(err, cmd) + require.Contains(t, errStr, "Run 'coder delete --help' for usage.") + }) +}