internal/assert: Better protobuf diffs by aliasing testify/assert

This change replaces the current `github.com/google/go-cmp/cmp` based internal `assert.Equal` with an aliased version of testify `assert.Equal`.
If we need other assertions, we'll need to
This allows better output for non-protobuf tests.

Unfortunately, testify does not have support for unmarshaling protobufs, so I needed to move to a new `assert.EqualProto` function that uses `protocmp` to compare the two protobufs.

This also exposed another issue that the old `assert.Equal` was working around: the Envoy DiscoveryResponse proto has version and nonce fields that can change in ways that make tests flaky.
However, it turns out that all the tests in `featuretests` that needed the workaround only change the `Resources` field.
So I've updated the featuretests infrastructure to have the `Equals` method on the Response struct for testing only compare that `Resources` field.
This should probably be replaced with `EqualResources` and `NoResources` for the normal and empty case respectively, but this PR is big enough for now.

Updates projectcontour#2786 for now, until a second PR that will change the names as above.

Signed-off-by: Nick Young <[email protected]>

cmd/contour/servecontext_test.go

@@ -470,7 +470,7 @@ func TestParseHTTPVersions(t *testing.T) {
 		},
 		"invalid proto": {
 			versions:      []string{"foo"},
-			parseError:    errors.New("invalid"),
+			parseError:    errors.New("invalid HTTP protocol version \"foo\""),
 			parseVersions: nil,
 		},
 		"http/1.1": {
@@ -501,8 +501,8 @@ func TestParseHTTPVersions(t *testing.T) {
 			sort.Slice(testcase.parseVersions,
 				func(i, j int) bool { return testcase.parseVersions[i] < testcase.parseVersions[j] })
 
-			assert.Equal(t, err, testcase.parseError)
-			assert.Equal(t, vers, testcase.parseVersions)
+			assert.Equal(t, testcase.parseError, err)
+			assert.Equal(t, testcase.parseVersions, vers)
 		})
 	}
 }

cmd/contour/shutdownmanager_test.go

@@ -88,13 +88,13 @@ func TestParseOpenConnections(t *testing.T) {
 	run(t, "missing values", testcase{
 		stats:           strings.NewReader(MISSING_STATS),
 		wantConnections: -1,
-		wantError:       fmt.Errorf("prometheus stat [envoy_http_downstream_cx_active] not found in request result"),
+		wantError:       fmt.Errorf("error finding Prometheus stat \"envoy_http_downstream_cx_active\" in the request result"),
 	})
 
 	run(t, "invalid stats", testcase{
 		stats:           strings.NewReader("!!##$$##!!"),
 		wantConnections: -1,
-		wantError:       fmt.Errorf("parsing prometheus text format failed: text format parsing error in line 1: invalid metric name"),
+		wantError:       fmt.Errorf("parsing Prometheus text format failed: text format parsing error in line 1: invalid metric name"),
 	})
 }
 

go.mod

@@ -9,8 +9,8 @@ require (
 	github.com/client9/misspell v0.3.4
 	github.com/envoyproxy/go-control-plane v0.9.5
 	github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
-	github.com/golang/protobuf v1.3.2
-	github.com/google/go-cmp v0.4.0
+	github.com/golang/protobuf v1.4.1
+	github.com/google/go-cmp v0.5.0
 	github.com/google/uuid v1.1.1
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
 	github.com/imdario/mergo v0.3.7 // indirect
@@ -19,9 +19,10 @@ require (
 	github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4
 	github.com/prometheus/common v0.6.0
 	github.com/sirupsen/logrus v1.4.2
-	github.com/stretchr/testify v1.5.1 // indirect
+	github.com/stretchr/testify v1.5.1
 	golang.org/x/tools v0.0.0-20190929041059-e7abfedfabcf // indirect
-	google.golang.org/grpc v1.25.1
+	google.golang.org/grpc v1.27.0
+	google.golang.org/protobuf v1.25.0
 	gopkg.in/alecthomas/kingpin.v2 v2.2.6
 	gopkg.in/yaml.v2 v2.2.8
 	k8s.io/api v0.18.2

go.sum

@@ -77,6 +77,7 @@ github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkg
 github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
 github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
+github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
 github.com/envoyproxy/go-control-plane v0.9.5 h1:lRJIqDD8yjV1YyPRqecMdytjDLs2fTXq363aCib5xPU=
 github.com/envoyproxy/go-control-plane v0.9.5/go.mod h1:OXl5to++W0ctG+EHWTFUjiypVxC/Y4VLc/KFU+al13s=
 github.com/envoyproxy/protoc-gen-validate v0.1.0 h1:EQciDnbrYxy13PgWoY8AqoxGiPrpgBZ1R8UNe3ddc+A=
@@ -174,12 +175,22 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
 github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs=
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
+github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8=
+github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:xKAWHe0F5eneWXFV3EuXVDTCmh+JuBKY0li0aMyXATA=
+github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrUpVNzEA03Pprs=
+github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w=
+github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0=
+github.com/golang/protobuf v1.4.1 h1:ZFgWrT+bLgsYPirOnRfKLYJLvssAegOj/hgyMFdJZe0=
+github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8=
 github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
 github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
+github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.0 h1:/QaMHBdZ26BB3SSst0Iwl10Epc+xhTquomWX0oZEB6w=
+github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/gofuzz v0.0.0-20161122191042-44d81051d367/go.mod h1:HP5RmnzzSNb993RKQDq4+1A4ia9nllfqcQFTQJedwGI=
 github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
@@ -508,12 +519,25 @@ google.golang.org/genproto v0.0.0-20190418145605-e7d98fc518a7/go.mod h1:VzzqZJRn
 google.golang.org/genproto v0.0.0-20190502173448-54afdca5d873/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE=
 google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 h1:gSJIx1SDwno+2ElGhA4+qG2zF97qiUzTM+rQ0klBOcE=
 google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc=
+google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 h1:+kGHl1aib/qcwaRi1CbqBZ1rk19r85MNUf8HaBghugY=
+google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.23.0 h1:AzbTB6ux+okLTzP8Ru1Xs41C303zdcfEht7MQnYJt5A=
 google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
 google.golang.org/grpc v1.23.1/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
 google.golang.org/grpc v1.25.1 h1:wdKvqQk7IttEw92GoRyKG2IDrUIpgpj6H6m81yfeMW0=
 google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
+google.golang.org/grpc v1.27.0 h1:rRYRFMVgRv6E0D70Skyfsr28tDXIuuPZyWGMPdMcnXg=
+google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
+google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
+google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
+google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=
+google.golang.org/protobuf v1.20.1-0.20200309200217-e05f789c0967/go.mod h1:A+miEFZTKqfCUM6K7xSMQL9OKL/b6hQv+e19PK+JZNE=
+google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzikPIcrTAo=
+google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
+google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
+google.golang.org/protobuf v1.25.0 h1:Ejskq+SyPohKW+1uil0JJMtmHCgJPJ/qWTxr8qp+R4c=
+google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
 gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc=
 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

internal/assert/assert.go

@@ -17,12 +17,13 @@ package assert
 import (
 	"testing"
 
-	v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
 	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes"
 	"github.com/golang/protobuf/ptypes/any"
 	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
+
+	tassert "github.com/stretchr/testify/assert"
+	"google.golang.org/protobuf/testing/protocmp"
 )
 
 type Assert struct {
@@ -39,14 +40,22 @@ func Equal(t *testing.T, want, got interface{}, opts ...cmp.Option) {
 
 // Equal will call t.Fatal if want and got are not equal.
 func (a Assert) Equal(want, got interface{}, opts ...cmp.Option) {
+	tassert.Equal(a.t, want, got, "Two values should be equal.")
+}
+
+// EqualProto will test that want == got, and call t.Fatal if it does not.
+// Notably, for errors, they are equal if they are both nil, or are both non-nil.
+// No value information is checked for errors.
+func EqualProto(t *testing.T, want, got interface{}, opts ...cmp.Option) {
+	t.Helper()
+	Assert{t}.EqualProto(want, got, opts...)
+}
+
+// EqualProto will call t.Fatal if want and got are not equal.
+func (a Assert) EqualProto(want, got interface{}, opts ...cmp.Option) {
 	a.t.Helper()
 	opts = append(opts,
-		cmpopts.IgnoreFields(v2.DiscoveryResponse{}, "VersionInfo", "Nonce"),
-		cmpopts.AcyclicTransformer("UnmarshalAny", unmarshalAny),
-		// errors to be equal only if both are nil or both are non-nil.
-		cmp.Comparer(func(x, y error) bool {
-			return (x == nil) == (y == nil)
-		}),
+		protocmp.Transform(),
 	)
 	diff := cmp.Diff(want, got, opts...)
 	if diff != "" {

internal/contour/cluster_test.go

@@ -71,7 +71,7 @@ func TestClusterCacheContents(t *testing.T) {
 			var cc ClusterCache
 			cc.Update(tc.contents)
 			got := cc.Contents()
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }
@@ -151,7 +151,7 @@ func TestClusterCacheQuery(t *testing.T) {
 			var cc ClusterCache
 			cc.Update(tc.contents)
 			got := cc.Query(tc.query)
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }

internal/contour/listener_test.go

@@ -66,7 +66,7 @@ func TestListenerCacheContents(t *testing.T) {
 			var lc ListenerCache
 			lc.Update(tc.contents)
 			got := lc.Contents()
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }
@@ -128,7 +128,7 @@ func TestListenerCacheQuery(t *testing.T) {
 			var lc ListenerCache
 			lc.Update(tc.contents)
 			got := lc.Query(tc.query)
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }

internal/contour/route_test.go

@@ -68,7 +68,7 @@ func TestRouteCacheContents(t *testing.T) {
 			var rc RouteCache
 			rc.Update(tc.contents)
 			got := rc.Contents()
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }
@@ -128,7 +128,7 @@ func TestRouteCacheQuery(t *testing.T) {
 			var rc RouteCache
 			rc.Update(tc.contents)
 			got := rc.Query(tc.query)
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }

internal/contour/secret_test.go

@@ -53,7 +53,7 @@ func TestSecretCacheContents(t *testing.T) {
 			var sc SecretCache
 			sc.Update(tc.contents)
 			got := sc.Contents()
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }
@@ -97,7 +97,7 @@ func TestSecretCacheQuery(t *testing.T) {
 			var sc SecretCache
 			sc.Update(tc.contents)
 			got := sc.Query(tc.query)
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }

internal/dag/secret_test.go

@@ -38,7 +38,7 @@ func TestIsValidSecret(t *testing.T) {
 			cert:  MISSING_CN_CERT,
 			key:   MISSING_CN_KEY,
 			valid: false,
-			err:   errors.New("certificate has no common name or subject alt name"),
+			err:   errors.New("invalid TLS certificate: certificate has no common name or subject alt name"),
 		},
 		"EC cert with SubjectAltName only": {
 			cert:  EC_CERTIFICATE,

internal/envoy/accesslog_test.go

@@ -104,7 +104,7 @@ func TestJSONFileAccessLog(t *testing.T) {
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			got := FileAccessLogJSON(tc.path, tc.headers)
-			assert.Equal(t, tc.want, got)
+			assert.EqualProto(t, tc.want, got)
 		})
 	}
 }

internal/envoy/bootstrap_test.go

@@ -1007,21 +1007,21 @@ func TestBootstrap(t *testing.T) {
 			if tc.wantedBootstrapConfig != "" {
 				want := new(envoy_api_bootstrap.Bootstrap)
 				unmarshal(t, tc.wantedBootstrapConfig, want)
-				assert.Equal(t, want, gotConfigs[tc.config.Path])
+				assert.EqualProto(t, want, gotConfigs[tc.config.Path])
 				delete(gotConfigs, tc.config.Path)
 			}
 
 			if tc.wantedTLSCertificateConfig != "" {
 				want := new(api.DiscoveryResponse)
 				unmarshal(t, tc.wantedTLSCertificateConfig, want)
-				assert.Equal(t, want, gotConfigs[sdsTLSCertificatePath])
+				assert.EqualProto(t, want, gotConfigs[sdsTLSCertificatePath])
 				delete(gotConfigs, sdsTLSCertificatePath)
 			}
 
 			if tc.wantedValidationContextConfig != "" {
 				want := new(api.DiscoveryResponse)
 				unmarshal(t, tc.wantedValidationContextConfig, want)
-				assert.Equal(t, want, gotConfigs[sdsValidationContextPath])
+				assert.EqualProto(t, want, gotConfigs[sdsValidationContextPath])
 				delete(gotConfigs, sdsValidationContextPath)
 			}
 

internal/envoy/healthcheck_test.go

@@ -18,7 +18,7 @@ import (
 	"time"
 
 	envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
-	"github.com/google/go-cmp/cmp"
+	"github.com/projectcontour/contour/internal/assert"
 	"github.com/projectcontour/contour/internal/dag"
 	"github.com/projectcontour/contour/internal/protobuf"
 )
@@ -95,9 +95,7 @@ func TestHealthCheck(t *testing.T) {
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			got := httpHealthCheck(tc.cluster)
-			if diff := cmp.Diff(tc.want, got); diff != "" {
-				t.Fatal(diff)
-			}
+			assert.Equal(t, tc.want, got)
 
 		})
 	}

internal/envoy/stats_test.go

@@ -21,7 +21,7 @@ import (
 	envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route"
 	http "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connection_manager/v2"
 	"github.com/envoyproxy/go-control-plane/pkg/wellknown"
-	"github.com/google/go-cmp/cmp"
+	"github.com/projectcontour/contour/internal/assert"
 	"github.com/projectcontour/contour/internal/protobuf"
 )
 
@@ -94,9 +94,7 @@ func TestStatsListener(t *testing.T) {
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			got := StatsListener(tc.address, tc.port)
-			if diff := cmp.Diff(tc.want, got); diff != "" {
-				t.Fatal(diff)
-			}
+			assert.Equal(t, tc.want, got)
 		})
 	}
 }

internal/featuretests/featuretests.go

@@ -384,7 +384,7 @@ type Response struct {
 
 func (r *Response) Equals(want *v2.DiscoveryResponse) *Contour {
 	r.Helper()
-	assert.Equal(r.T, want, r.DiscoveryResponse)
+	assert.EqualProto(r.T, want.Resources, r.DiscoveryResponse.Resources)
 
 	return r.Contour
 }