wire/test: support multi-line errors and -record mode in tests (google/go-cloud#550)

internal/wire/analyze.go

@@ -20,6 +20,7 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
+	"sort"
 	"strings"
 
 	"golang.org/x/tools/go/types/typeutil"
@@ -381,7 +382,10 @@ func verifyAcyclic(providerMap *typeutil.Map, hasher typeutil.Hasher) []error {
 	visited := new(typeutil.Map) // to bool
 	visited.SetHasher(hasher)
 	ec := new(errorCollector)
-	for _, root := range providerMap.Keys() {
+	// Sort output types so that errors about cycles are consistent.
+	outputs := providerMap.Keys()
+	sort.Slice(outputs, func(i, j int) bool { return types.TypeString(outputs[i], nil) < types.TypeString(outputs[j], nil) })
+	for _, root := range outputs {
 		// Depth-first search using a stack of trails through the provider map.
 		stk := [][]types.Type{{root}}
 		for len(stk) > 0 {

internal/wire/testdata/Cycle/want/wire_errs.txt

@@ -1 +1,5 @@
-cycle
+/wire_gopath/src/example.com/foo/wire.go:x:y: cycle for example.com/foo.Bar:
+example.com/foo.Bar (example.com/foo.provideBar) ->
+example.com/foo.Foo (example.com/foo.provideFoo) ->
+example.com/foo.Baz (example.com/foo.provideBaz) ->
+example.com/foo.Bar

internal/wire/testdata/EmptyVar/want/wire_errs.txt

@@ -1 +1 @@
-not a provider
+/wire_gopath/src/example.com/foo/wire.go:x:y: var example.com/foo.myFakeSet struct{} is not a provider or a provider set

internal/wire/testdata/InjectInputConflict/want/wire_errs.txt

@@ -1,2 +1 @@
-foo.Foo
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: input of example.com/foo.Foo conflicts with provider provideFoo at /wire_gopath/src/example.com/foo/foo.go:x:y
-conflicts with provider

internal/wire/testdata/InjectorMissingCleanup/want/wire_errs.txt

@@ -1 +1 @@
-provider for example.com/foo.Foo returns cleanup but injection does not return cleanup function
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns cleanup but injection does not return cleanup function

internal/wire/testdata/InjectorMissingError/want/wire_errs.txt

@@ -1 +1 @@
-provider for example.com/foo.Foo returns error but injection not allowed to fail
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns error but injection not allowed to fail

internal/wire/testdata/InterfaceBindingDoesntImplement/want/wire_errs.txt

@@ -1 +1 @@
-string does not implement example.com/foo.Fooer
+/wire_gopath/src/example.com/foo/wire.go:x:y: string does not implement example.com/foo.Fooer

internal/wire/testdata/InterfaceBindingInvalidArg0/want/wire_errs.txt

@@ -1 +1 @@
-first argument to Bind must be a pointer to an interface type; found string
+/wire_gopath/src/example.com/foo/wire.go:x:y: first argument to Bind must be a pointer to an interface type; found string

internal/wire/testdata/InterfaceBindingNotEnoughArgs/want/wire_errs.txt

@@ -1 +1 @@
-too few arguments in call to wire.Bind
+/wire_gopath/src/example.com/foo/wire.go:x:y: too few arguments in call to wire.Bind

internal/wire/testdata/InterfaceValueDoesntImplement/want/wire_errs.txt

@@ -1 +1 @@
-string does not implement io.Reader
+/wire_gopath/src/example.com/foo/wire.go:x:y: string does not implement io.Reader

internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt

@@ -1 +1 @@
-first argument to InterfaceValue must be a pointer to an interface type; found string
+/wire_gopath/src/example.com/foo/wire.go:x:y: first argument to InterfaceValue must be a pointer to an interface type; found string

internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt

@@ -1 +1 @@
-too few arguments in call to wire.InterfaceValue
+/wire_gopath/src/example.com/foo/wire.go:x:y: too few arguments in call to wire.InterfaceValue

internal/wire/testdata/InvalidInjector/want/wire_errs.txt

@@ -1,4 +1,3 @@
-a call to wire.Build indicates that this function is an injector, but injectors
+a call to wire.Build indicates that this function is an injector, but injectors must consist of only the wire.Build call and an optional return
-must consist of only the wire.Build call and an optional return
+
-a call to wire.Build indicates that this function is an injector, but injectors
+a call to wire.Build indicates that this function is an injector, but injectors must consist of only the wire.Build call and an optional return
-must consist of only the wire.Build call and an optional return

internal/wire/testdata/MultipleBindings/want/wire_errs.txt

@@ -1,6 +1,11 @@
-/wire_gopath/src/example.com/foo/wire.go:27:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFooAgain" (/wire_gopath/src/example.com/foo/foo.go:39:6); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6)
+/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFooAgain" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
-/wire_gopath/src/example.com/foo/wire.go:32:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11)
+
-/wire_gopath/src/example.com/foo/wire.go:37:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:32:16)
+/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y)
-/wire_gopath/src/example.com/foo/foo.go:33:32: SetWithDuplicateBindings has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:32:16); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11)
+
-/wire_gopath/src/example.com/foo/wire.go:47:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: wire.Value (/wire_gopath/src/example.com/foo/wire.go:47:42); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6)
+/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:x:y)
-/wire_gopath/src/example.com/foo/wire.go:52:8: wire.Build has multiple bindings for example.com/foo.Bar (current binding: wire.Bind (/wire_gopath/src/example.com/foo/wire.go:52:31); previous binding: provider "provideBar" (/wire_gopath/src/example.com/foo/foo.go:43:6)
+
+/wire_gopath/src/example.com/foo/foo.go:x:y: SetWithDuplicateBindings has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y)
+
+/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: wire.Value (/wire_gopath/src/example.com/foo/wire.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y)
+
+/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Bar (current binding: wire.Bind (/wire_gopath/src/example.com/foo/wire.go:x:y); previous binding: provider "provideBar" (/wire_gopath/src/example.com/foo/foo.go:x:y)

internal/wire/testdata/MultipleMissingInputs/want/wire_errs.txt

@@ -1,4 +1,7 @@
-inject injectMissingOutputType: no provider found for example.com/foo.Foo, output of injector
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMissingOutputType: no provider found for example.com/foo.Foo, output of injector
-inject injectMultipleMissingTypes: no provider found for example.com/foo.Foo, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:29:6)
+
-inject injectMultipleMissingTypes: no provider found for example.com/foo.Bar, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:29:6)
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Foo, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:x:y)
-inject injectMissingRecursiveType: no provider found for example.com/foo.Foo, needed by example.com/foo.Zip in provider "provideZip" (/wire_gopath/src/example.com/foo/foo.go:37:6), needed by example.com/foo.Zap in provider "provideZap" (/wire_gopath/src/example.com/foo/foo.go:41:6), needed by example.com/foo.Zop in provider "provideZop" (/wire_gopath/src/example.com/foo/foo.go:45:6)
+
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Bar, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:x:y)
+
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMissingRecursiveType: no provider found for example.com/foo.Foo, needed by example.com/foo.Zip in provider "provideZip" (/wire_gopath/src/example.com/foo/foo.go:x:y), needed by example.com/foo.Zap in provider "provideZap" (/wire_gopath/src/example.com/foo/foo.go:x:y), needed by example.com/foo.Zop in provider "provideZop" (/wire_gopath/src/example.com/foo/foo.go:x:y)

internal/wire/testdata/NoImplicitInterface/want/wire_errs.txt

@@ -1,2 +1 @@
-no provider found
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFooer: no provider found for example.com/foo.Fooer, output of injector
-Fooer

internal/wire/testdata/UnexportedValue/want/wire_errs.txt

@@ -1 +1 @@
-unexported identifier privateMsg
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectedMessage: value string can't be used: uses unexported identifier privateMsg

internal/wire/testdata/UnusedProviders/want/wire_errs.txt

@@ -1,4 +1,7 @@
-unused provider set "unusedSet"
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused provider set "unusedSet"
-unused provider "provideUnused"
+
-unused value of type string
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused provider "provideUnused"
-unused interface binding to type example.com/foo.Fooer
+
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused value of type string
+
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused interface binding to type example.com/foo.Fooer

internal/wire/testdata/ValueFromFunctionScope/want/wire_errs.txt

@@ -1 +1 @@
-inject injectBar: value int can't be used: f is not declared in package scope
+/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: value int can't be used: f is not declared in package scope

internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt

@@ -1 +1 @@
-argument to Value may not be an interface value (found io.Reader); use InterfaceValue instead
+/wire_gopath/src/example.com/foo/wire.go:x:y: argument to Value may not be an interface value (found io.Reader); use InterfaceValue instead

internal/wire/wire_test.go

@@ -25,6 +25,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"sort"
 	"strings"
@@ -81,15 +82,22 @@ func TestWire(t *testing.T) {
 				defer t.Logf("wire_gen.go:\n%s", gen)
 			}
 			if len(errs) > 0 {
-				for _, e := range errs {
+				gotErrStrings := make([]string, len(errs))
-					t.Log(e)
+				for i, e := range errs {
+					gotErrStrings[i] = scrubError(e.Error())
+					t.Log(gotErrStrings[i])
 				}
 				if !test.wantWireError {
-					t.Fatal("Did not expect errors.")
+					t.Fatal("Did not expect errors. To -record an error, create want/wire_errs.txt.")
 				}
-				for _, s := range test.wantWireErrorStrings {
+				if *setup.Record {
-					if !errorListContains(errs, s) {
+					wireErrsFile := filepath.Join(testRoot, test.name, "want", "wire_errs.txt")
-						t.Errorf("Errors did not contain %q", s)
+					if err := ioutil.WriteFile(wireErrsFile, []byte(strings.Join(gotErrStrings, "\n\n")), 0666); err != nil {
+						t.Fatalf("failed to write wire_errs.txt file: %v", err)
+					}
+				} else {
+					if diff := cmp.Diff(gotErrStrings, test.wantWireErrorStrings); diff != "" {
+						t.Errorf("Errors didn't match expected errors from wire_errors.txt:\n%s", diff)
 					}
 				}
 				return
@@ -359,6 +367,14 @@ type testCase struct {
 	wantWireErrorStrings []string
 }
 
+var scrubLineNumberAndPositionRegex = regexp.MustCompile("\\.go:[\\d]+:[\\d]+")
+var scrubLineNumberRegex = regexp.MustCompile("\\.go:[\\d]+")
+
+func scrubError(s string) string {
+	s = scrubLineNumberAndPositionRegex.ReplaceAllString(s, ".go:x:y")
+	return scrubLineNumberRegex.ReplaceAllString(s, ".go:x")
+}
+
 // loadTestCase reads a test case from a directory.
 //
 // The directory structure is:
@@ -375,8 +391,11 @@ type testCase struct {
 //		want/
 //
 //			wire_errs.txt
-//					expected errors from the Wire Generate function,
+//					Expected errors from the Wire Generate function,
-//					missing if no errors expected
+//					missing if no errors expected.
+//					Distinct errors are separated by a blank line,
+//					and line numbers and line positions are scrubbed
+//					(e.g., "foo.go:52:8" --> "foo.go:x:y").
 //
 //			wire_gen.go
 //					verified output of wire from a test run with
@@ -398,7 +417,7 @@ func loadTestCase(root string, wireGoSrc []byte) (*testCase, error) {
 	wantWireError := err == nil
 	var wantWireErrorStrings []string
 	if wantWireError {
-		wantWireErrorStrings = strings.Split(strings.TrimSpace(string(wireErrb)), "\n")
+		wantWireErrorStrings = strings.Split(scrubError(string(wireErrb)), "\n\n")
 	} else {
 		if !*setup.Record {
 			wantWireOutput, err = ioutil.ReadFile(filepath.Join(root, "want", "wire_gen.go"))
@@ -691,12 +710,3 @@ func runGo(bctx *build.Context, dir string, args ...string) error {
 	}
 	return nil
 }
-
-func errorListContains(errs []error, substr string) bool {
-	for _, e := range errs {
-		if strings.Contains(e.Error(), substr) {
-			return true
-		}
-	}
-	return false
-}