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

Changes from 1 commit
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
Prev Previous commit
Next Next commit
fixup! refactor(cli): show --help error message on main
  • Loading branch information
@jsjoeio
jsjoeio committed May 19, 2022
commit a7bd7bd7b6372b03068cc7cdaec11ee5550a71ba
8 changes: 5 additions & 3 deletions cli/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
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")

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