Skip to content

Commit

Permalink
xds: Add cmpopts.EquateEmpty option in tests. (#3671)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@easwars
easwars committed Jun 10, 2020
1 parent 479df5e commit d5bc6ec
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 16 deletions.
19 changes: 11 additions & 8 deletions xds/internal/client/client_watchers_endpoints_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -42,6 +44,7 @@ var (
Weight: 1,
},
}
endpointsCmpOpts = []cmp.Option{cmp.AllowUnexported(endpointsUpdateErr{}), cmpopts.EquateEmpty()}
)

type endpointsUpdateErr struct {
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 12 additions & 8 deletions xds/internal/client/client_watchers_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
}

0 comments on commit d5bc6ec

Please sign in to comment.