Skip to content

fix: show --help message for CLI errors, add tests for delete #1403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 19, 2022
Merged

fix: show --help message for CLI errors, add tests for delete #1403

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
758dceb
feat(cli): add test for delete
jsjoeio May 19, 2022
6ff347e
fix(cli): use ExecuteC() to match Cobra
jsjoeio May 19, 2022
16c19c7
feat(cli): add WithoutParameters test for delete
jsjoeio May 19, 2022
dae45ab
fixup! feat(cli): add WithoutParameters test for delete
jsjoeio May 19, 2022
32cbc85
refactor(cli): show --help error message on main
jsjoeio May 19, 2022
a7bd7bd
fixup! refactor(cli): show --help error message on main
jsjoeio May 19, 2022
c28e93a
refactor(cli): handle err with FormatCobraError
jsjoeio May 19, 2022
8ac2198
refactor(cli): add root_test.go, move delete test
jsjoeio May 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions cli/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package cli_test

import (
"testing"

"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"
)

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
})
t.Run("WithoutParameters/FormatCobraError", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: nothing wrong with this test case that I can see, but since it's just testing the FormatCobraError function and doesn't have anything to do with the functionality of coder delete, it seems a bit odd for it to live in this file. How about creating root_test.go and putting it there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! @f0ssel and I were discussing that as well and we had landed on leaving here but happy to move it!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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.")
})
}
7 changes: 7 additions & 0 deletions cli/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"fmt"
"net/url"
"os"
"time"
Expand Down Expand Up @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted into function to make it easier to test

helpErrMsg := fmt.Sprintf("Run '%s %s --help' for usage.", cmd.Root().Name(), cmd.Name())
return cliui.Styles.Error.Render(err.Error() + "\n" + helpErrMsg)
}
5 changes: 3 additions & 2 deletions cmd/coder/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ import (

func main() {
dadjoke()
err := cli.Root().Execute()
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()))
cobraErr := cli.FormatCobraError(err, cmd)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this behavior mimics what SilenceErrors: true does, which prints the --help message below any errors. See this comment from @mafredri for more context: #1403 (comment)

_, _ = fmt.Fprintln(os.Stderr, cobraErr)
os.Exit(1)
}
}
Expand Down