From d5bc6ecb590760edf37158ab03fb7c83962e3d5a Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Wed, 10 Jun 2020 08:53:19 -0700 Subject: [PATCH] xds: Add cmpopts.EquateEmpty option in tests. (#3671) Recently I have started seeing a lot of xds tests fail in travis when doing cmp.Equal. Adding the EquateEmpty option will treat all maps and slices of length zero as equal whether they are empty or nil. --- .../client/client_watchers_endpoints_test.go | 19 ++++++++++-------- .../client/client_watchers_service_test.go | 20 +++++++++++-------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/xds/internal/client/client_watchers_endpoints_test.go b/xds/internal/client/client_watchers_endpoints_test.go index 53b1bc316ab..e61bb710523 100644 --- a/xds/internal/client/client_watchers_endpoints_test.go +++ b/xds/internal/client/client_watchers_endpoints_test.go @@ -23,6 +23,8 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "google.golang.org/grpc/xds/internal" "google.golang.org/grpc/xds/internal/testutils" ) @@ -42,6 +44,7 @@ var ( Weight: 1, }, } + endpointsCmpOpts = []cmp.Option{cmp.AllowUnexported(endpointsUpdateErr{}), cmpopts.EquateEmpty()} ) type endpointsUpdateErr struct { @@ -78,7 +81,7 @@ func (s) TestEndpointsWatch(t *testing.T) { testCDSName: wantUpdate, }) - if u, err := endpointsUpdateCh.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if u, err := endpointsUpdateCh.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, endpointsCmpOpts...) { t.Errorf("unexpected endpointsUpdate: %v, error receiving from channel: %v", u, err) } @@ -138,7 +141,7 @@ func (s) TestEndpointsTwoWatchSameResourceName(t *testing.T) { }) for i := 0; i < count; i++ { - if u, err := endpointsUpdateChs[i].Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if u, err := endpointsUpdateChs[i].Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, endpointsCmpOpts...) { t.Errorf("i=%v, unexpected endpointsUpdate: %v, error receiving from channel: %v", i, u, err) } } @@ -150,7 +153,7 @@ func (s) TestEndpointsTwoWatchSameResourceName(t *testing.T) { }) for i := 0; i < count-1; i++ { - if u, err := endpointsUpdateChs[i].Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if u, err := endpointsUpdateChs[i].Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, endpointsCmpOpts...) { t.Errorf("i=%v, unexpected endpointsUpdate: %v, error receiving from channel: %v", i, u, err) } } @@ -206,12 +209,12 @@ func (s) TestEndpointsThreeWatchDifferentResourceName(t *testing.T) { }) for i := 0; i < count; i++ { - if u, err := endpointsUpdateChs[i].Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate1, nil}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if u, err := endpointsUpdateChs[i].Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate1, nil}, endpointsCmpOpts...) { t.Errorf("i=%v, unexpected endpointsUpdate: %v, error receiving from channel: %v", i, u, err) } } - if u, err := endpointsUpdateCh2.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate2, nil}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if u, err := endpointsUpdateCh2.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate2, nil}, endpointsCmpOpts...) { t.Errorf("unexpected endpointsUpdate: %v, error receiving from channel: %v", u, err) } } @@ -243,7 +246,7 @@ func (s) TestEndpointsWatchAfterCache(t *testing.T) { testCDSName: wantUpdate, }) - if u, err := endpointsUpdateCh.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if u, err := endpointsUpdateCh.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, endpointsCmpOpts...) { t.Errorf("unexpected endpointsUpdate: %v, error receiving from channel: %v", u, err) } @@ -257,7 +260,7 @@ func (s) TestEndpointsWatchAfterCache(t *testing.T) { } // New watch should receives the update. - if u, err := endpointsUpdateCh2.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if u, err := endpointsUpdateCh2.Receive(); err != nil || !cmp.Equal(u, endpointsUpdateErr{wantUpdate, nil}, endpointsCmpOpts...) { t.Errorf("unexpected endpointsUpdate: %v, error receiving from channel: %v", u, err) } @@ -301,7 +304,7 @@ func (s) TestEndpointsWatchExpiryTimer(t *testing.T) { t.Fatalf("failed to get endpointsUpdate: %v", err) } uu := u.(endpointsUpdateErr) - if !cmp.Equal(uu.u, EndpointsUpdate{}, cmp.AllowUnexported(endpointsUpdateErr{})) { + if !cmp.Equal(uu.u, EndpointsUpdate{}, endpointsCmpOpts...) { t.Errorf("unexpected endpointsUpdate: %v, want %v", uu.u, EndpointsUpdate{}) } if uu.err == nil { diff --git a/xds/internal/client/client_watchers_service_test.go b/xds/internal/client/client_watchers_service_test.go index 8b341683d09..37d7fb45401 100644 --- a/xds/internal/client/client_watchers_service_test.go +++ b/xds/internal/client/client_watchers_service_test.go @@ -25,6 +25,8 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "google.golang.org/grpc/xds/internal/testutils" "google.golang.org/grpc/xds/internal/testutils/fakeserver" ) @@ -34,6 +36,8 @@ type serviceUpdateErr struct { err error } +var serviceCmpOpts = []cmp.Option{cmp.AllowUnexported(serviceUpdateErr{}), cmpopts.EquateEmpty()} + // TestServiceWatch covers the cases: // - an update is received after a watch() // - an update for another resource name (which doesn't trigger callback) @@ -70,7 +74,7 @@ func (s) TestServiceWatch(t *testing.T) { testRDSName: {weightedCluster: map[string]uint32{testCDSName: 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } } @@ -110,7 +114,7 @@ func (s) TestServiceWatchLDSUpdate(t *testing.T) { testRDSName: {weightedCluster: map[string]uint32{testCDSName: 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } @@ -137,7 +141,7 @@ func (s) TestServiceWatchLDSUpdate(t *testing.T) { testRDSName + "2": {weightedCluster: map[string]uint32{testCDSName + "2": 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate2, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate2, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } } @@ -177,7 +181,7 @@ func (s) TestServiceWatchSecond(t *testing.T) { testRDSName: {weightedCluster: map[string]uint32{testCDSName: 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } @@ -208,7 +212,7 @@ func (s) TestServiceWatchSecond(t *testing.T) { testRDSName: {weightedCluster: map[string]uint32{testCDSName: 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } @@ -388,7 +392,7 @@ func (s) TestServiceNotCancelRDSOnSameLDSUpdate(t *testing.T) { testRDSName: {weightedCluster: map[string]uint32{testCDSName: 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } @@ -439,7 +443,7 @@ func (s) TestServiceResourceRemoved(t *testing.T) { testRDSName: {weightedCluster: map[string]uint32{testCDSName: 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{wantUpdate, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } @@ -478,7 +482,7 @@ func (s) TestServiceResourceRemoved(t *testing.T) { v2Client.r.newRDSUpdate(map[string]rdsUpdate{ testRDSName: {weightedCluster: map[string]uint32{testCDSName + "new2": 1}}, }) - if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{ServiceUpdate{WeightedCluster: map[string]uint32{testCDSName + "new2": 1}}, nil}, cmp.AllowUnexported(serviceUpdateErr{})) { + if u, err := serviceUpdateCh.Receive(); err != nil || !cmp.Equal(u, serviceUpdateErr{ServiceUpdate{WeightedCluster: map[string]uint32{testCDSName + "new2": 1}}, nil}, serviceCmpOpts...) { t.Errorf("unexpected serviceUpdate: %v, error receiving from channel: %v", u, err) } }