Skip to content

Commit c6e6634

Browse files
committed
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. fix: prevent data race refactor: make formatter public Parse error command in a less brittle way
1 parent 1d2d008 commit c6e6634

11 files changed

+168
-48
lines changed

Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ update-golden-files: \
642642
.PHONY: update-golden-files
643643

644644
cli/testdata/.gen-golden: $(wildcard cli/testdata/*.golden) $(wildcard cli/*.tpl) $(GO_SRC_FILES) $(wildcard cli/*_test.go)
645-
go test ./cli -run="Test(CommandHelp|ServerYAML)" -update
645+
go test ./cli -run="Test(CommandHelp|ServerYAML|ErrorExamples)" -update
646646
touch "$@"
647647

648648
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

+31-26
Original file line numberDiff line numberDiff line change
@@ -87,40 +87,45 @@ ExtractCommandPathsLoop:
8787

8888
StartWithWaiter(t, inv.WithContext(ctx)).RequireSuccess()
8989

90-
actual := outBuf.Bytes()
91-
if len(actual) == 0 {
92-
t.Fatal("no output")
93-
}
94-
95-
for k, v := range replacements {
96-
actual = bytes.ReplaceAll(actual, []byte(k), []byte(v))
97-
}
90+
TestGoldenFile(t, tt.Name, outBuf.Bytes(), replacements)
91+
})
92+
}
93+
}
9894

99-
actual = NormalizeGoldenFile(t, actual)
100-
goldenPath := filepath.Join("testdata", strings.Replace(tt.Name, " ", "_", -1)+".golden")
101-
if *UpdateGoldenFiles {
102-
t.Logf("update golden file for: %q: %s", tt.Name, goldenPath)
103-
err := os.WriteFile(goldenPath, actual, 0o600)
104-
require.NoError(t, err, "update golden file")
105-
}
95+
// TestGoldenFile will test the given bytes slice input against the
96+
// golden file with the given file name, optionally using the given replacements.
97+
func TestGoldenFile(t *testing.T, fileName string, actual []byte, replacements map[string]string) {
98+
if len(actual) == 0 {
99+
t.Fatal("no output")
100+
}
106101

107-
expected, err := os.ReadFile(goldenPath)
108-
require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes")
102+
for k, v := range replacements {
103+
actual = bytes.ReplaceAll(actual, []byte(k), []byte(v))
104+
}
109105

110-
expected = NormalizeGoldenFile(t, expected)
111-
require.Equal(
112-
t, string(expected), string(actual),
113-
"golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes",
114-
goldenPath,
115-
)
116-
})
106+
actual = normalizeGoldenFile(t, actual)
107+
goldenPath := filepath.Join("testdata", strings.ReplaceAll(fileName, " ", "_")+".golden")
108+
if *UpdateGoldenFiles {
109+
t.Logf("update golden file for: %q: %s", fileName, goldenPath)
110+
err := os.WriteFile(goldenPath, actual, 0o600)
111+
require.NoError(t, err, "update golden file")
117112
}
113+
114+
expected, err := os.ReadFile(goldenPath)
115+
require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes")
116+
117+
expected = normalizeGoldenFile(t, expected)
118+
require.Equal(
119+
t, string(expected), string(actual),
120+
"golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes",
121+
goldenPath,
122+
)
118123
}
119124

120-
// NormalizeGoldenFile replaces any strings that are system or timing dependent
125+
// normalizeGoldenFile replaces any strings that are system or timing dependent
121126
// with a placeholder so that the golden files can be compared with a simple
122127
// equality check.
123-
func NormalizeGoldenFile(t *testing.T, byt []byte) []byte {
128+
func normalizeGoldenFile(t *testing.T, byt []byte) []byte {
124129
// Replace any timestamps with a placeholder.
125130
byt = timestampRegex.ReplaceAll(byt, []byte("[timestamp]"))
126131

cli/errors_test.go

+100
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package cli_test
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"os"
7+
"strings"
8+
"testing"
9+
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/coder/coder/v2/cli"
13+
"github.com/coder/coder/v2/cli/clitest"
14+
"github.com/coder/serpent"
15+
)
16+
17+
type commandErrorCase struct {
18+
Name string
19+
Cmd []string
20+
}
21+
22+
// TestErrorExamples will test the help output of the
23+
// coder exp example-error using golden files.
24+
func TestErrorExamples(t *testing.T) {
25+
t.Parallel()
26+
27+
coderRootCmd := getRoot(t)
28+
29+
var exampleErrorRootCmd *serpent.Command
30+
coderRootCmd.Walk(func(command *serpent.Command) {
31+
if command.Name() == "example-error" {
32+
// cannot abort early, but list is small
33+
exampleErrorRootCmd = command
34+
}
35+
})
36+
require.NotNil(t, exampleErrorRootCmd, "example-error command not found")
37+
38+
var cases []commandErrorCase
39+
40+
ExtractCommandPathsLoop:
41+
for _, cp := range extractCommandPaths(nil, exampleErrorRootCmd.Children) {
42+
cmd := append([]string{"exp", "example-error"}, cp...)
43+
name := fmt.Sprintf("coder %s", strings.Join(cmd, " "))
44+
for _, tt := range cases {
45+
if tt.Name == name {
46+
continue ExtractCommandPathsLoop
47+
}
48+
}
49+
cases = append(cases, commandErrorCase{Name: name, Cmd: cmd})
50+
}
51+
52+
for _, tt := range cases {
53+
tt := tt
54+
t.Run(tt.Name, func(t *testing.T) {
55+
t.Parallel()
56+
57+
var outBuf bytes.Buffer
58+
59+
coderRootCmd := getRoot(t)
60+
61+
inv, _ := clitest.NewWithCommand(t, coderRootCmd, tt.Cmd...)
62+
inv.Stderr = &outBuf
63+
inv.Stdout = &outBuf
64+
65+
// This example expects to close stdin twice and joins
66+
// the error messages to create a multi-multi error.
67+
if tt.Name == "coder exp example-error multi-multi-error" {
68+
inv.Stdin = os.Stdin
69+
}
70+
71+
err := inv.Run()
72+
73+
errFormatter := cli.NewPrettyErrorFormatter(&outBuf, false)
74+
errFormatter.Format(err)
75+
76+
clitest.TestGoldenFile(t, tt.Name, outBuf.Bytes(), nil)
77+
})
78+
}
79+
}
80+
81+
func extractCommandPaths(cmdPath []string, cmds []*serpent.Command) [][]string {
82+
var cmdPaths [][]string
83+
for _, c := range cmds {
84+
cmdPath := append(cmdPath, c.Name())
85+
cmdPaths = append(cmdPaths, cmdPath)
86+
cmdPaths = append(cmdPaths, extractCommandPaths(cmdPath, c.Children)...)
87+
}
88+
return cmdPaths
89+
}
90+
91+
// Must return a fresh instance of cmds each time.
92+
func getRoot(t *testing.T) *serpent.Command {
93+
t.Helper()
94+
95+
var root cli.RootCmd
96+
rootCmd, err := root.Command(root.AGPL())
97+
require.NoError(t, err)
98+
99+
return rootCmd
100+
}

cli/root.go

+14-6
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ func (r *RootCmd) RunWithSubcommands(subcommands []*serpent.Command) {
167167
//nolint:revive
168168
os.Exit(code)
169169
}
170-
f := prettyErrorFormatter{w: os.Stderr, verbose: r.verbose}
170+
f := PrettyErrorFormatter{w: os.Stderr, verbose: r.verbose}
171171
if err != nil {
172-
f.format(err)
172+
f.Format(err)
173173
}
174174
//nolint:revive
175175
os.Exit(code)
@@ -909,15 +909,23 @@ func ExitError(code int, err error) error {
909909
return &exitError{code: code, err: err}
910910
}
911911

912-
type prettyErrorFormatter struct {
912+
// NewPrettyErrorFormatter creates a new PrettyErrorFormatter.
913+
func NewPrettyErrorFormatter(w io.Writer, verbose bool) *PrettyErrorFormatter {
914+
return &PrettyErrorFormatter{
915+
w: w,
916+
verbose: verbose,
917+
}
918+
}
919+
920+
type PrettyErrorFormatter struct {
913921
w io.Writer
914922
// verbose turns on more detailed error logs, such as stack traces.
915923
verbose bool
916924
}
917925

918-
// format formats the error to the console. This error should be human
919-
// readable.
920-
func (p *prettyErrorFormatter) format(err error) {
926+
// Format formats the error to the writer in PrettyErrorFormatter.
927+
// This error should be human readable.
928+
func (p *PrettyErrorFormatter) Format(err error) {
921929
output, _ := cliHumanFormatError("", err, &formatOpts{
922930
Verbose: p.verbose,
923931
})

cli/server_test.go

+1-15
Original file line numberDiff line numberDiff line change
@@ -1774,21 +1774,7 @@ func TestServerYAMLConfig(t *testing.T) {
17741774
err = enc.Encode(n)
17751775
require.NoError(t, err)
17761776

1777-
wantByt := wantBuf.Bytes()
1778-
1779-
goldenPath := filepath.Join("testdata", "server-config.yaml.golden")
1780-
1781-
wantByt = clitest.NormalizeGoldenFile(t, wantByt)
1782-
if *clitest.UpdateGoldenFiles {
1783-
require.NoError(t, os.WriteFile(goldenPath, wantByt, 0o600))
1784-
return
1785-
}
1786-
1787-
got, err := os.ReadFile(goldenPath)
1788-
require.NoError(t, err)
1789-
got = clitest.NormalizeGoldenFile(t, got)
1790-
1791-
require.Equal(t, string(wantByt), string(got))
1777+
clitest.TestGoldenFile(t, "server-config.yaml", wantBuf.Bytes(), nil)
17921778
}
17931779

17941780
func TestConnectToPostgres(t *testing.T) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Encountered an error running "coder exp example-error api", see "coder exp example-error api --help" for more information
2+
error: Top level sdk error message.
3+
Have you tried turning it off and on again?
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Encountered an error running "coder exp example-error arg-required", see "coder exp example-error arg-required --help" for more information
2+
error: wanted 1 args but got 0 []
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Encountered an error running "coder exp example-error cmd", see "coder exp example-error cmd --help" for more information
2+
error: some error: function decided not to work, and it never will
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Encountered an error running "coder exp example-error multi-error", see "coder exp example-error multi-error --help" for more information
2+
error: 3 errors encountered: Trace=[wrapped: ])
3+
1. first error: function decided not to work, and it never will
4+
2. second error: function decided not to work, and it never will
5+
3. Trace=[wrapped api error: ]
6+
Top level sdk error message.
7+
magic dust unavailable, please try again later
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2 errors encountered:
2+
1. Encountered an error running "coder exp example-error multi-multi-error", see "coder exp example-error multi-multi-error --help" for more information
3+
error: 2 errors encountered:
4+
1. first error: function decided not to work, and it never will
5+
2. second error: function decided not to work, and it never will
6+
2. close /dev/stdin: file already closed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Missing values for the required flags: magic-word

0 commit comments

Comments
 (0)