From 1d225cf371d9ef55b29ae7e69fb48ab0cb77e437 Mon Sep 17 00:00:00 2001 From: Ross Light Date: Thu, 28 Jun 2018 12:53:30 -0700 Subject: [PATCH] wire: add error string assertions in tests (google/go-cloud#149) out.txts that start with "ERROR" can now include substrings to assert exist in the error messages, one per line. This gives a very coarse-grained way of testing for error message quality. It is not a substitute for manual verification, but gives a "good enough" sanity check that relevant details are reported. Updates google/go-cloud#5 --- internal/wire/testdata/Cycle/out.txt | 1 + internal/wire/testdata/EmptyVar/out.txt | 1 + .../wire/testdata/InjectInputConflict/out.txt | 2 + .../wire/testdata/NoImplicitInterface/out.txt | 2 + .../wire/testdata/UnexportedValue/out.txt | 1 + internal/wire/wire_test.go | 58 +++++++++++++++---- 6 files changed, 53 insertions(+), 12 deletions(-) diff --git a/internal/wire/testdata/Cycle/out.txt b/internal/wire/testdata/Cycle/out.txt index 5df7507..d48ed29 100644 --- a/internal/wire/testdata/Cycle/out.txt +++ b/internal/wire/testdata/Cycle/out.txt @@ -1 +1,2 @@ ERROR +cycle diff --git a/internal/wire/testdata/EmptyVar/out.txt b/internal/wire/testdata/EmptyVar/out.txt index 5df7507..d5ddc95 100644 --- a/internal/wire/testdata/EmptyVar/out.txt +++ b/internal/wire/testdata/EmptyVar/out.txt @@ -1 +1,2 @@ ERROR +not a provider diff --git a/internal/wire/testdata/InjectInputConflict/out.txt b/internal/wire/testdata/InjectInputConflict/out.txt index 5df7507..285f317 100644 --- a/internal/wire/testdata/InjectInputConflict/out.txt +++ b/internal/wire/testdata/InjectInputConflict/out.txt @@ -1 +1,3 @@ ERROR +foo.Foo +conflicts with provider diff --git a/internal/wire/testdata/NoImplicitInterface/out.txt b/internal/wire/testdata/NoImplicitInterface/out.txt index 5df7507..61b82b7 100644 --- a/internal/wire/testdata/NoImplicitInterface/out.txt +++ b/internal/wire/testdata/NoImplicitInterface/out.txt @@ -1 +1,3 @@ ERROR +no provider found +Fooer diff --git a/internal/wire/testdata/UnexportedValue/out.txt b/internal/wire/testdata/UnexportedValue/out.txt index 5df7507..6b1466c 100644 --- a/internal/wire/testdata/UnexportedValue/out.txt +++ b/internal/wire/testdata/UnexportedValue/out.txt @@ -1 +1,2 @@ ERROR +unexported identifier privateMsg diff --git a/internal/wire/wire_test.go b/internal/wire/wire_test.go index 08cd07e..9019a5a 100644 --- a/internal/wire/wire_test.go +++ b/internal/wire/wire_test.go @@ -74,8 +74,16 @@ func TestWire(t *testing.T) { defer t.Logf("wire_gen.go:\n%s", gen) } if len(errs) > 0 { + for _, e := range errs { + t.Log(e) + } if !test.wantError { - t.Fatalf("wirego: %v", errs) + t.Fatal("Did not expect errors.") + } + for _, s := range test.wantErrorStrings { + if !errorListContains(errs, s) { + t.Errorf("Errors did not contain %q", s) + } } return } @@ -269,7 +277,9 @@ type testCase struct { pkg string goFiles map[string][]byte wantOutput []byte - wantError bool + + wantError bool + wantErrorStrings []string } // loadTestCase reads a test case from a directory. @@ -279,8 +289,10 @@ type testCase struct { // root/ // pkg file containing the package name containing the inject function // (must also be package main) -// out.txt file containing the expected output, or the magic string "ERROR" -// if this test should cause generation to fail +// out.txt file containing the expected output, or starting with the +// magic line "ERROR" if this test should cause generation to +// fail (any subsequent lines are substrings that should +// appear in the errors). // ... any Go files found recursively placed under GOPATH/src/... func loadTestCase(root string, wireGoSrc []byte) (*testCase, error) { name := filepath.Base(root) @@ -292,9 +304,8 @@ func loadTestCase(root string, wireGoSrc []byte) (*testCase, error) { if err != nil { return nil, fmt.Errorf("load test case %s: %v", name, err) } - wantError := false - if bytes.Equal(bytes.TrimSpace(out), []byte("ERROR")) { - wantError = true + wantErrorStrings, wantError := parseGoldenOutput(out) + if wantError { out = nil } goFiles := map[string][]byte{ @@ -322,11 +333,12 @@ func loadTestCase(root string, wireGoSrc []byte) (*testCase, error) { return nil, fmt.Errorf("load test case %s: %v", name, err) } return &testCase{ - name: name, - pkg: string(bytes.TrimSpace(pkg)), - goFiles: goFiles, - wantOutput: out, - wantError: wantError, + name: name, + pkg: string(bytes.TrimSpace(pkg)), + goFiles: goFiles, + wantOutput: out, + wantError: wantError, + wantErrorStrings: wantErrorStrings, }, nil } @@ -576,3 +588,25 @@ func runDiff(a, b []byte) ([]byte, error) { out, err := c.Output() return out, err } + +func parseGoldenOutput(out []byte) (errorStrings []string, wantError bool) { + const errorPrefix = "ERROR\n" + if !bytes.HasPrefix(out, []byte(errorPrefix)) { + return nil, false + } + // Skip past first line. + out = out[len(errorPrefix):] + // Remove any leading or trailing blank lines. + out = bytes.Trim(out[len(errorPrefix):], "\n") + // Split lines. + return strings.Split(string(out), "\n"), true +} + +func errorListContains(errs []error, substr string) bool { + for _, e := range errs { + if strings.Contains(e.Error(), substr) { + return true + } + } + return false +}