Better compare TabletStats in tests.

go/vt/discovery/healthcheck.go

@@ -29,6 +29,7 @@ import (
 	"time"
 
 	log "github.com/golang/glog"
+	"github.com/golang/protobuf/proto"
 	"github.com/youtube/vitess/go/netutil"
 	"github.com/youtube/vitess/go/stats"
 	querypb "github.com/youtube/vitess/go/vt/proto/query"
@@ -148,6 +149,21 @@ func (e *TabletStats) String() string {
 	return fmt.Sprint(*e)
 }
 
+// DeepEqual compares two TabletStats. Since we include protos, we
+// need to use proto.Equal on these.
+func (e *TabletStats) DeepEqual(f *TabletStats) bool {
+	return e.Key == f.Key &&
+		proto.Equal(e.Tablet, f.Tablet) &&
+		e.Name == f.Name &&
+		proto.Equal(e.Target, f.Target) &&
+		e.Up == f.Up &&
+		e.Serving == f.Serving &&
+		e.TabletExternallyReparentedTimestamp == f.TabletExternallyReparentedTimestamp &&
+		proto.Equal(e.Stats, f.Stats) &&
+		((e.LastError == nil && f.LastError == nil) ||
+			(e.LastError != nil && f.LastError != nil && e.LastError.Error() == f.LastError.Error()))
+}
+
 // HealthCheck defines the interface of health checking module.
 // The goal of this object is to maintain a StreamHealth RPC
 // to a lot of tablets. Tablets are added / removed by calling the

go/vt/discovery/replicationlag_test.go

@@ -1,7 +1,6 @@
 package discovery
 
 import (
-	"reflect"
 	"testing"
 
 	querypb "github.com/youtube/vitess/go/vt/proto/query"
@@ -29,7 +28,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 	if len(got) != 1 {
 		t.Errorf("len(FilterByReplicationLag([{Tablet: {Uid: 1}, Serving: true}, {Tablet: {Uid: 2}, Serving: false}])) = %v, want 1", len(got))
 	}
-	if len(got) > 0 && !reflect.DeepEqual(got[0], ts1) {
+	if len(got) > 0 && !got[0].DeepEqual(ts1) {
 		t.Errorf("FilterByReplicationLag([{Tablet: {Uid: 1}, Serving: true}, {Tablet: {Uid: 2}, Serving: false}]) = %+v, want %+v", got[0], ts1)
 	}
 	// lags of (1s, 1s, 1s, 30s)
@@ -54,7 +53,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 		Stats:   &querypb.RealtimeStats{SecondsBehindMaster: 30},
 	}
 	got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4})
-	if len(got) != 4 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) || !reflect.DeepEqual(got[3], ts4) {
+	if len(got) != 4 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) || !got[3].DeepEqual(ts4) {
 		t.Errorf("FilterByReplicationLag([1s, 1s, 1s, 30s]) = %+v, want all", got)
 	}
 	// lags of (5s, 10s, 15s, 120s)
@@ -79,7 +78,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 		Stats:   &querypb.RealtimeStats{SecondsBehindMaster: 120},
 	}
 	got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4})
-	if len(got) != 3 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) {
+	if len(got) != 3 || !got[0].DeepEqual(got[0]) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) {
 		t.Errorf("FilterByReplicationLag([5s, 10s, 15s, 120s]) = %+v, want [5s, 10s, 15s]", got)
 	}
 	// lags of (30m, 35m, 40m, 45m)
@@ -104,7 +103,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 		Stats:   &querypb.RealtimeStats{SecondsBehindMaster: 45 * 60},
 	}
 	got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4})
-	if len(got) != 4 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) || !reflect.DeepEqual(got[3], ts4) {
+	if len(got) != 4 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) || !got[3].DeepEqual(ts4) {
 		t.Errorf("FilterByReplicationLag([30m, 35m, 40m, 45m]) = %+v, want all", got)
 	}
 	// lags of (1s, 1s, 1m, 40m, 40m) - not run filter the second time as first run removed two items.
@@ -134,7 +133,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 		Stats:   &querypb.RealtimeStats{SecondsBehindMaster: 40 * 60},
 	}
 	got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4, ts5})
-	if len(got) != 3 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) || !reflect.DeepEqual(got[2], ts3) {
+	if len(got) != 3 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) || !got[2].DeepEqual(ts3) {
 		t.Errorf("FilterByReplicationLag([1s, 1s, 1m, 40m, 40m]) = %+v, want [1s, 1s, 1m]", got)
 	}
 	// lags of (1s, 1s, 10m, 40m) - run filter twice to remove two items
@@ -159,7 +158,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 		Stats:   &querypb.RealtimeStats{SecondsBehindMaster: 40 * 60},
 	}
 	got = FilterByReplicationLag([]*TabletStats{ts1, ts2, ts3, ts4})
-	if len(got) != 2 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) {
+	if len(got) != 2 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) {
 		t.Errorf("FilterByReplicationLag([1s, 1s, 10m, 40m]) = %+v, want [1s, 1s]", got)
 	}
 	// lags of (1m, 100m) - return at least 2 items to avoid overloading if the 2nd one is not delayed too much
@@ -174,7 +173,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 		Stats:   &querypb.RealtimeStats{SecondsBehindMaster: 100 * 60},
 	}
 	got = FilterByReplicationLag([]*TabletStats{ts1, ts2})
-	if len(got) != 2 || !reflect.DeepEqual(got[0], ts1) || !reflect.DeepEqual(got[1], ts2) {
+	if len(got) != 2 || !got[0].DeepEqual(ts1) || !got[1].DeepEqual(ts2) {
 		t.Errorf("FilterByReplicationLag([1m, 100m]) = %+v, want all", got)
 	}
 	// lags of (1m, 3h) - return 1 if the 2nd one is delayed too much
@@ -189,7 +188,7 @@ func TestFilterByReplicationLag(t *testing.T) {
 		Stats:   &querypb.RealtimeStats{SecondsBehindMaster: 3 * 60 * 60},
 	}
 	got = FilterByReplicationLag([]*TabletStats{ts1, ts2})
-	if len(got) != 1 || !reflect.DeepEqual(got[0], ts1) {
+	if len(got) != 1 || !got[0].DeepEqual(ts1) {
 		t.Errorf("FilterByReplicationLag([1m, 3h]) = %+v, want [1m]", got)
 	}
 }

go/vt/discovery/tablet_stats_cache_test.go

@@ -1,7 +1,6 @@
 package discovery
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/youtube/vitess/go/vt/topo"
@@ -40,11 +39,11 @@ func TestTabletStatsCache(t *testing.T) {
 
 	// check it's there
 	a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 	a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 
@@ -61,11 +60,11 @@ func TestTabletStatsCache(t *testing.T) {
 
 	// check the previous ts1 is still there, as the new one is ignored.
 	a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 	a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 
@@ -82,11 +81,11 @@ func TestTabletStatsCache(t *testing.T) {
 
 	// check it's there
 	a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*notHealthyTs1, a[0]) {
+	if len(a) != 1 || !notHealthyTs1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 	a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*notHealthyTs1, a[0]) {
+	if len(a) != 1 || !notHealthyTs1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 
@@ -110,7 +109,7 @@ func TestTabletStatsCache(t *testing.T) {
 		if a[0].Tablet.Alias.Uid == 11 {
 			a[0], a[1] = a[1], a[0]
 		}
-		if !reflect.DeepEqual(*ts1, a[0]) || !reflect.DeepEqual(*ts2, a[1]) {
+		if !ts1.DeepEqual(&a[0]) || !ts2.DeepEqual(&a[1]) {
 			t.Errorf("unexpected result: %v", a)
 		}
 	}
@@ -121,7 +120,7 @@ func TestTabletStatsCache(t *testing.T) {
 		if a[0].Tablet.Alias.Uid == 11 {
 			a[0], a[1] = a[1], a[0]
 		}
-		if !reflect.DeepEqual(*ts1, a[0]) || !reflect.DeepEqual(*ts2, a[1]) {
+		if !ts1.DeepEqual(&a[0]) || !ts2.DeepEqual(&a[1]) {
 			t.Errorf("unexpected result: %v", a)
 		}
 	}
@@ -138,12 +137,12 @@ func TestTabletStatsCache(t *testing.T) {
 		if a[0].Tablet.Alias.Uid == 11 {
 			a[0], a[1] = a[1], a[0]
 		}
-		if !reflect.DeepEqual(*ts1, a[0]) || !reflect.DeepEqual(*ts2, a[1]) {
+		if !ts1.DeepEqual(&a[0]) || !ts2.DeepEqual(&a[1]) {
 			t.Errorf("unexpected result: %v", a)
 		}
 	}
 	a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 
@@ -158,13 +157,13 @@ func TestTabletStatsCache(t *testing.T) {
 
 	// check we only have one replica left
 	a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_REPLICA)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 
 	// check we have a master now
 	a = tsc.GetTabletStats("k", "s", topodatapb.TabletType_MASTER)
-	if len(a) != 1 || !reflect.DeepEqual(*ts2, a[0]) {
+	if len(a) != 1 || !ts2.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 
@@ -182,14 +181,14 @@ func TestTabletStatsCache(t *testing.T) {
 		t.Errorf("unexpected result: %v", a)
 	}
 	a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_MASTER)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 
 	// old master sending an old ping should be ignored
 	tsc.StatsUpdate(ts2)
 	a = tsc.GetHealthyTabletStats("k", "s", topodatapb.TabletType_MASTER)
-	if len(a) != 1 || !reflect.DeepEqual(*ts1, a[0]) {
+	if len(a) != 1 || !ts1.DeepEqual(&a[0]) {
 		t.Errorf("unexpected result: %v", a)
 	}
 }

go/vt/discovery/utils_test.go

@@ -1,7 +1,6 @@
 package discovery
 
 import (
-	"reflect"
 	"testing"
 
 	querypb "github.com/youtube/vitess/go/vt/proto/query"
@@ -47,8 +46,15 @@ func TestRemoveUnhealthyTablets(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		if got := RemoveUnhealthyTablets(tc.input); !reflect.DeepEqual(got, tc.want) {
+		got := RemoveUnhealthyTablets(tc.input)
+		if len(got) != len(tc.want) {
 			t.Errorf("test case '%v' failed: RemoveUnhealthyTablets(%v) = %#v, want: %#v", tc.desc, tc.input, got, tc.want)
+		} else {
+			for i := range tc.want {
+				if !got[i].DeepEqual(&tc.want[i]) {
+					t.Errorf("test case '%v' failed: RemoveUnhealthyTablets(%v) = %#v, want: %#v", tc.desc, tc.input, got, tc.want)
+				}
+			}
 		}
 	}
 }

go/vt/vtctld/realtime_status_test.go

@@ -2,7 +2,6 @@ package vtctld
 
 import (
 	"fmt"
-	"reflect"
 	"testing"
 	"time"
 
@@ -106,7 +105,7 @@ func checkStats(realtimeStats *realtimeStats, tablet *testlib.FakeTablet, want *
 		if err != nil {
 			continue
 		}
-		if reflect.DeepEqual(result, discovery.TabletStats{}) {
+		if result.DeepEqual(&discovery.TabletStats{}) {
 			continue
 		}
 		got := result.Stats

go/vt/vtctld/tablet_stats_cache_test.go

@@ -22,26 +22,26 @@ func TestStatsUpdate(t *testing.T) {
 	// Insert tablet1.
 	tabletStatsCache.StatsUpdate(tablet1Stats1)
 	results1 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA]
-	if got, want := results1[0], tablet1Stats1; !reflect.DeepEqual(got, want) {
+	if got, want := results1[0], tablet1Stats1; !got.DeepEqual(want) {
 		t.Errorf("got: %v, want: %v", got, want)
 	}
 
 	// Update tablet1.
 	tabletStatsCache.StatsUpdate(tablet1Stats2)
 	results2 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA]
-	if got, want := results2[0], tablet1Stats2; !reflect.DeepEqual(got, want) {
+	if got, want := results2[0], tablet1Stats2; !got.DeepEqual(want) {
 		t.Errorf("got: %v, want: %v", got, want)
 	}
 
 	// Insert tablet. List of tablets will be resorted.
 	tabletStatsCache.StatsUpdate(tablet2Stats1)
 	results3 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA]
-	if got, want := results3[0], tablet2Stats1; !reflect.DeepEqual(got, want) {
+	if got, want := results3[0], tablet2Stats1; !got.DeepEqual(want) {
 		t.Errorf("got: %v, want: %v", got, want)
 	}
 
 	results4 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA]
-	if got, want := results4[1], tablet1Stats2; !reflect.DeepEqual(got, want) {
+	if got, want := results4[1], tablet1Stats2; !got.DeepEqual(want) {
 		t.Errorf("got: %v, want: %v", got, want)
 	}
 
@@ -50,7 +50,7 @@ func TestStatsUpdate(t *testing.T) {
 	tabletStatsCache.StatsUpdate(tablet2Stats1)
 	results5 := tabletStatsCache.statuses["ks1"]["-80"]["cell1"][topodatapb.TabletType_REPLICA]
 	for _, stat := range results5 {
-		if reflect.DeepEqual(stat, tablet2Stats1) {
+		if stat.DeepEqual(tablet2Stats1) {
 			t.Errorf("not deleleted from statusesByAliases")
 		}
 	}
@@ -329,14 +329,14 @@ func TestTabletStats(t *testing.T) {
 
 	// Test 1: tablet1 and tablet2 are updated with the stats received by the HealthCheck module.
 	got1, err := tabletStatsCache.tabletStats(ts1.Tablet.Alias)
-	want1 := *ts1
-	if err != nil || !reflect.DeepEqual(got1, want1) {
+	want1 := ts1
+	if err != nil || !got1.DeepEqual(want1) {
 		t.Errorf("got: %v, want: %v", got1, want1)
 	}
 
 	got2, err := tabletStatsCache.tabletStats(ts2.Tablet.Alias)
-	want2 := *ts2
-	if err != nil || !reflect.DeepEqual(got2, want2) {
+	want2 := ts2
+	if err != nil || !got2.DeepEqual(want2) {
 		t.Errorf("got: %v, want: %v", got2, want2)
 	}