Skip to content

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

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.

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 2 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
57 changes: 31 additions & 26 deletions cli/clitest/golden.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
TestGoldenFile(t, tt.Name, outBuf.Bytes(), replacements)
})
}
}

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

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

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,
)
})
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
// 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]"))

Expand Down
14 changes: 5 additions & 9 deletions cli/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"os"

"golang.org/x/xerrors"

Expand Down Expand Up @@ -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()),
),
)
},
},
Expand Down
93 changes: 93 additions & 0 deletions cli/errors_test.go
Original file line number Diff line number Diff line change
@@ -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
}
20 changes: 14 additions & 6 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
})
Expand Down
16 changes: 1 addition & 15 deletions cli/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions cli/testdata/coder_exp_example-error_api.golden
Original file line number Diff line number Diff line change
@@ -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?
2 changes: 2 additions & 0 deletions cli/testdata/coder_exp_example-error_arg-required.golden
Original file line number Diff line number Diff line change
@@ -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 []
2 changes: 2 additions & 0 deletions cli/testdata/coder_exp_example-error_cmd.golden
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions cli/testdata/coder_exp_example-error_multi-error.golden
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions cli/testdata/coder_exp_example-error_multi-multi-error.golden
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions cli/testdata/coder_exp_example-error_validation.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Missing values for the required flags: magic-word