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 5 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
46 changes: 46 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,46 @@
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
})
t.Run("WithoutParameters", func(t *testing.T) {
t.Parallel()

cmd, _ := clitest.New(t, "delete")

err := cmd.Execute()
require.ErrorContains(t, err, "Run 'coder delete --help' for usage.")
})
}
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()))
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))
Copy link
Member

Choose a reason for hiding this comment

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

Most errors are not usage errors and shouldn't show the usage. Stuff like HTTP errors would trigger this for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True! @f0ssel mentioned the same thing. Maybe that's why SilenceErrors is set to true?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way we can detect a cobra error like errors.Is(err, cobra.ErrArgs) or something?

Copy link
Contributor Author

@jsjoeio jsjoeio May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I looked around the docs but didn't find anything. Seems like it just returns a regular error. Maybe someone sees something though.

os.Exit(1)
}
}
Expand Down