feat(cli): add golden tests for errors (#11588) (#12698)

* feat(cli): add golden tests for errors (#11588)

Creates golden files from `coder/cli/errors.go`.
Adds a unit test to test against golden files.
Adds a make file command to regenerate golden files.
Abstracts test against golden files.

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)

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

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()),
+						),
 					)
 				},
 			},

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
+}

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,
 	})

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

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?

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 []

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

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

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

cli/testdata/coder_exp_example-error_validation.golden

@@ -0,0 +1 @@
+Missing values for the required flags: magic-word