diff --git a/Makefile b/Makefile index 4aabb3951f..84a97323cd 100644 --- a/Makefile +++ b/Makefile @@ -642,7 +642,7 @@ update-golden-files: \ .PHONY: update-golden-files cli/testdata/.gen-golden: $(wildcard cli/testdata/*.golden) $(wildcard cli/*.tpl) $(GO_SRC_FILES) $(wildcard cli/*_test.go) - go test ./cli -run="Test(CommandHelp|ServerYAML)" -update + go test ./cli -run="Test(CommandHelp|ServerYAML|ErrorExamples)" -update touch "$@" enterprise/cli/testdata/.gen-golden: $(wildcard enterprise/cli/testdata/*.golden) $(wildcard cli/*.tpl) $(GO_SRC_FILES) $(wildcard enterprise/cli/*_test.go) diff --git a/cli/clitest/golden.go b/cli/clitest/golden.go index eec504e70b..635ced97d4 100644 --- a/cli/clitest/golden.go +++ b/cli/clitest/golden.go @@ -87,40 +87,45 @@ ExtractCommandPathsLoop: StartWithWaiter(t, inv.WithContext(ctx)).RequireSuccess() - actual := outBuf.Bytes() - if len(actual) == 0 { - t.Fatal("no output") - } - - for k, v := range replacements { - actual = bytes.ReplaceAll(actual, []byte(k), []byte(v)) - } - - actual = NormalizeGoldenFile(t, actual) - goldenPath := filepath.Join("testdata", strings.Replace(tt.Name, " ", "_", -1)+".golden") - if *UpdateGoldenFiles { - t.Logf("update golden file for: %q: %s", tt.Name, goldenPath) - err := os.WriteFile(goldenPath, actual, 0o600) - require.NoError(t, err, "update golden file") - } - - expected, err := os.ReadFile(goldenPath) - require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes") - - expected = NormalizeGoldenFile(t, expected) - require.Equal( - t, string(expected), string(actual), - "golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes", - goldenPath, - ) + TestGoldenFile(t, tt.Name, outBuf.Bytes(), replacements) }) } } -// NormalizeGoldenFile replaces any strings that are system or timing dependent +// TestGoldenFile will test the given bytes slice input against the +// golden file with the given file name, optionally using the given replacements. +func TestGoldenFile(t *testing.T, fileName string, actual []byte, replacements map[string]string) { + if len(actual) == 0 { + t.Fatal("no output") + } + + for k, v := range replacements { + actual = bytes.ReplaceAll(actual, []byte(k), []byte(v)) + } + + actual = normalizeGoldenFile(t, actual) + goldenPath := filepath.Join("testdata", strings.ReplaceAll(fileName, " ", "_")+".golden") + if *UpdateGoldenFiles { + t.Logf("update golden file for: %q: %s", fileName, goldenPath) + err := os.WriteFile(goldenPath, actual, 0o600) + require.NoError(t, err, "update golden file") + } + + expected, err := os.ReadFile(goldenPath) + require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes") + + expected = normalizeGoldenFile(t, expected) + require.Equal( + t, string(expected), string(actual), + "golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes", + goldenPath, + ) +} + +// normalizeGoldenFile replaces any strings that are system or timing dependent // with a placeholder so that the golden files can be compared with a simple // equality check. -func NormalizeGoldenFile(t *testing.T, byt []byte) []byte { +func normalizeGoldenFile(t *testing.T, byt []byte) []byte { // Replace any timestamps with a placeholder. byt = timestampRegex.ReplaceAll(byt, []byte("[timestamp]")) diff --git a/cli/errors.go b/cli/errors.go index 414474c3ae..fbcaf8091c 100644 --- a/cli/errors.go +++ b/cli/errors.go @@ -5,7 +5,6 @@ import ( "fmt" "net/http" "net/http/httptest" - "os" "golang.org/x/xerrors" @@ -83,15 +82,12 @@ func (RootCmd) errorExample() *serpent.Command { Use: "multi-multi-error", Short: "This is a multi error inside a multi error", Handler: func(inv *serpent.Invocation) error { - // Closing the stdin file descriptor will cause the next close - // to fail. This is joined to the returned Command error. - if f, ok := inv.Stdin.(*os.File); ok { - _ = f.Close() - } - return errors.Join( - xerrors.Errorf("first error: %w", errorWithStackTrace()), - xerrors.Errorf("second error: %w", errorWithStackTrace()), + xerrors.Errorf("parent error: %w", errorWithStackTrace()), + errors.Join( + xerrors.Errorf("child first error: %w", errorWithStackTrace()), + xerrors.Errorf("child second error: %w", errorWithStackTrace()), + ), ) }, }, diff --git a/cli/errors_test.go b/cli/errors_test.go new file mode 100644 index 0000000000..75272fc86d --- /dev/null +++ b/cli/errors_test.go @@ -0,0 +1,93 @@ +package cli_test + +import ( + "bytes" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/cli" + "github.com/coder/coder/v2/cli/clitest" + "github.com/coder/serpent" +) + +type commandErrorCase struct { + Name string + Cmd []string +} + +// TestErrorExamples will test the help output of the +// coder exp example-error using golden files. +func TestErrorExamples(t *testing.T) { + t.Parallel() + + coderRootCmd := getRoot(t) + + var exampleErrorRootCmd *serpent.Command + coderRootCmd.Walk(func(command *serpent.Command) { + if command.Name() == "example-error" { + // cannot abort early, but list is small + exampleErrorRootCmd = command + } + }) + require.NotNil(t, exampleErrorRootCmd, "example-error command not found") + + var cases []commandErrorCase + +ExtractCommandPathsLoop: + for _, cp := range extractCommandPaths(nil, exampleErrorRootCmd.Children) { + cmd := append([]string{"exp", "example-error"}, cp...) + name := fmt.Sprintf("coder %s", strings.Join(cmd, " ")) + for _, tt := range cases { + if tt.Name == name { + continue ExtractCommandPathsLoop + } + } + cases = append(cases, commandErrorCase{Name: name, Cmd: cmd}) + } + + for _, tt := range cases { + tt := tt + t.Run(tt.Name, func(t *testing.T) { + t.Parallel() + + var outBuf bytes.Buffer + + coderRootCmd := getRoot(t) + + inv, _ := clitest.NewWithCommand(t, coderRootCmd, tt.Cmd...) + inv.Stderr = &outBuf + inv.Stdout = &outBuf + + err := inv.Run() + + errFormatter := cli.NewPrettyErrorFormatter(&outBuf, false) + errFormatter.Format(err) + + clitest.TestGoldenFile(t, tt.Name, outBuf.Bytes(), nil) + }) + } +} + +func extractCommandPaths(cmdPath []string, cmds []*serpent.Command) [][]string { + var cmdPaths [][]string + for _, c := range cmds { + cmdPath := append(cmdPath, c.Name()) + cmdPaths = append(cmdPaths, cmdPath) + cmdPaths = append(cmdPaths, extractCommandPaths(cmdPath, c.Children)...) + } + return cmdPaths +} + +// Must return a fresh instance of cmds each time. +func getRoot(t *testing.T) *serpent.Command { + t.Helper() + + var root cli.RootCmd + rootCmd, err := root.Command(root.AGPL()) + require.NoError(t, err) + + return rootCmd +} diff --git a/cli/root.go b/cli/root.go index bbc59a8d94..83367169df 100644 --- a/cli/root.go +++ b/cli/root.go @@ -167,9 +167,9 @@ func (r *RootCmd) RunWithSubcommands(subcommands []*serpent.Command) { //nolint:revive os.Exit(code) } - f := prettyErrorFormatter{w: os.Stderr, verbose: r.verbose} + f := PrettyErrorFormatter{w: os.Stderr, verbose: r.verbose} if err != nil { - f.format(err) + f.Format(err) } //nolint:revive os.Exit(code) @@ -909,15 +909,23 @@ func ExitError(code int, err error) error { return &exitError{code: code, err: err} } -type prettyErrorFormatter struct { +// NewPrettyErrorFormatter creates a new PrettyErrorFormatter. +func NewPrettyErrorFormatter(w io.Writer, verbose bool) *PrettyErrorFormatter { + return &PrettyErrorFormatter{ + w: w, + verbose: verbose, + } +} + +type PrettyErrorFormatter struct { w io.Writer // verbose turns on more detailed error logs, such as stack traces. verbose bool } -// format formats the error to the console. This error should be human -// readable. -func (p *prettyErrorFormatter) format(err error) { +// Format formats the error to the writer in PrettyErrorFormatter. +// This error should be human readable. +func (p *PrettyErrorFormatter) Format(err error) { output, _ := cliHumanFormatError("", err, &formatOpts{ Verbose: p.verbose, }) diff --git a/cli/server_test.go b/cli/server_test.go index 21fd076787..7842f9e62b 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -1774,21 +1774,7 @@ func TestServerYAMLConfig(t *testing.T) { err = enc.Encode(n) require.NoError(t, err) - wantByt := wantBuf.Bytes() - - goldenPath := filepath.Join("testdata", "server-config.yaml.golden") - - wantByt = clitest.NormalizeGoldenFile(t, wantByt) - if *clitest.UpdateGoldenFiles { - require.NoError(t, os.WriteFile(goldenPath, wantByt, 0o600)) - return - } - - got, err := os.ReadFile(goldenPath) - require.NoError(t, err) - got = clitest.NormalizeGoldenFile(t, got) - - require.Equal(t, string(wantByt), string(got)) + clitest.TestGoldenFile(t, "server-config.yaml", wantBuf.Bytes(), nil) } func TestConnectToPostgres(t *testing.T) { diff --git a/cli/testdata/coder_exp_example-error_api.golden b/cli/testdata/coder_exp_example-error_api.golden new file mode 100644 index 0000000000..e15b60abbd --- /dev/null +++ b/cli/testdata/coder_exp_example-error_api.golden @@ -0,0 +1,3 @@ +Encountered an error running "coder exp example-error api", see "coder exp example-error api --help" for more information +error: Top level sdk error message. +Have you tried turning it off and on again? diff --git a/cli/testdata/coder_exp_example-error_arg-required.golden b/cli/testdata/coder_exp_example-error_arg-required.golden new file mode 100644 index 0000000000..fdb5264072 --- /dev/null +++ b/cli/testdata/coder_exp_example-error_arg-required.golden @@ -0,0 +1,2 @@ +Encountered an error running "coder exp example-error arg-required", see "coder exp example-error arg-required --help" for more information +error: wanted 1 args but got 0 [] diff --git a/cli/testdata/coder_exp_example-error_cmd.golden b/cli/testdata/coder_exp_example-error_cmd.golden new file mode 100644 index 0000000000..aaae237095 --- /dev/null +++ b/cli/testdata/coder_exp_example-error_cmd.golden @@ -0,0 +1,2 @@ +Encountered an error running "coder exp example-error cmd", see "coder exp example-error cmd --help" for more information +error: some error: function decided not to work, and it never will diff --git a/cli/testdata/coder_exp_example-error_multi-error.golden b/cli/testdata/coder_exp_example-error_multi-error.golden new file mode 100644 index 0000000000..73a32afd80 --- /dev/null +++ b/cli/testdata/coder_exp_example-error_multi-error.golden @@ -0,0 +1,7 @@ +Encountered an error running "coder exp example-error multi-error", see "coder exp example-error multi-error --help" for more information +error: 3 errors encountered: Trace=[wrapped: ]) +1. first error: function decided not to work, and it never will +2. second error: function decided not to work, and it never will +3. Trace=[wrapped api error: ] + Top level sdk error message. + magic dust unavailable, please try again later diff --git a/cli/testdata/coder_exp_example-error_multi-multi-error.golden b/cli/testdata/coder_exp_example-error_multi-multi-error.golden new file mode 100644 index 0000000000..029710e7d4 --- /dev/null +++ b/cli/testdata/coder_exp_example-error_multi-multi-error.golden @@ -0,0 +1,6 @@ +Encountered an error running "coder exp example-error multi-multi-error", see "coder exp example-error multi-multi-error --help" for more information +error: 2 errors encountered: +1. parent error: function decided not to work, and it never will +2. 2 errors encountered: + 1. child first error: function decided not to work, and it never will + 2. child second error: function decided not to work, and it never will diff --git a/cli/testdata/coder_exp_example-error_validation.golden b/cli/testdata/coder_exp_example-error_validation.golden new file mode 100644 index 0000000000..02f24e23e1 --- /dev/null +++ b/cli/testdata/coder_exp_example-error_validation.golden @@ -0,0 +1 @@ +Missing values for the required flags: magic-word