From 2d2f65684c31db528091b6d8cbf81c3ac9e06084 Mon Sep 17 00:00:00 2001
From: Joe Tsai <joetsai@digital-static.net>
Date: Wed, 6 Nov 2019 14:25:07 -0800
Subject: [PATCH] cleanup: fix generic comparisons on protobuf messages (#3153)

Generated protobuf messages contain internal data structures
that general purpose comparison functions (e.g., reflect.DeepEqual,
pretty.Compare, etc) do not properly compare. It is already the case
today that these functions may report a difference when two messages
are actually semantically equivalent.

Fix all usages by either calling proto.Equal directly if
the top-level types are themselves proto.Message, or by calling
cmp.Equal with the cmp.Comparer(proto.Equal) option specified.
This option teaches cmp to use proto.Equal anytime it encounters
proto.Message types.
---
 status/status_test.go                 | 8 ++++++--
 test/end2end_test.go                  | 6 +++++-
 test/retry_test.go                    | 3 +--
 xds/internal/balancer/lrs/lrs_test.go | 2 +-
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/status/status_test.go b/status/status_test.go
index b7db2d3d..a1932aa6 100644
--- a/status/status_test.go
+++ b/status/status_test.go
@@ -22,13 +22,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"reflect"
 	"testing"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes"
 	apb "github.com/golang/protobuf/ptypes/any"
 	dpb "github.com/golang/protobuf/ptypes/duration"
+	"github.com/google/go-cmp/cmp"
 	cpb "google.golang.org/genproto/googleapis/rpc/code"
 	epb "google.golang.org/genproto/googleapis/rpc/errdetails"
 	spb "google.golang.org/genproto/googleapis/rpc/status"
@@ -318,12 +318,16 @@ func TestStatus_ErrorDetails_Fail(t *testing.T) {
 	}
 	for _, tc := range tests {
 		got := tc.s.Details()
-		if !reflect.DeepEqual(got, tc.i) {
+		if !cmp.Equal(got, tc.i, cmp.Comparer(proto.Equal), cmp.Comparer(equalError)) {
 			t.Errorf("(%v).Details() = %+v, want %+v", str(tc.s), got, tc.i)
 		}
 	}
 }
 
+func equalError(x, y error) bool {
+	return x == y || (x != nil && y != nil && x.Error() == y.Error())
+}
+
 func str(s *Status) string {
 	if s == nil {
 		return "nil"
diff --git a/test/end2end_test.go b/test/end2end_test.go
index 61ca9bc8..f9491739 100644
--- a/test/end2end_test.go
+++ b/test/end2end_test.go
@@ -3919,11 +3919,15 @@ func testFailedServerStreaming(t *testing.T, e env) {
 		t.Fatalf("%v.StreamingOutputCall(_) = _, %v, want <nil>", tc, err)
 	}
 	wantErr := status.Error(codes.DataLoss, "error for testing: "+failAppUA)
-	if _, err := stream.Recv(); !reflect.DeepEqual(err, wantErr) {
+	if _, err := stream.Recv(); !equalError(err, wantErr) {
 		t.Fatalf("%v.Recv() = _, %v, want _, %v", stream, err, wantErr)
 	}
 }
 
+func equalError(x, y error) bool {
+	return x == y || (x != nil && y != nil && x.Error() == y.Error())
+}
+
 // concurrentSendServer is a TestServiceServer whose
 // StreamingOutputCall makes ten serial Send calls, sending payloads
 // "0".."9", inclusive.  TestServerStreamingConcurrent verifies they
diff --git a/test/retry_test.go b/test/retry_test.go
index 0e8fd0e0..f0ab380e 100644
--- a/test/retry_test.go
+++ b/test/retry_test.go
@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"reflect"
 	"strconv"
 	"strings"
 	"testing"
@@ -362,7 +361,7 @@ func (s) TestRetryStreaming(t *testing.T) {
 			res, err := stream.Recv()
 			if res != nil ||
 				((err == nil) != (want == nil)) ||
-				(want != nil && !reflect.DeepEqual(err, want)) {
+				(want != nil && err.Error() != want.Error()) {
 				return fmt.Errorf("client: Recv() = %v, %v; want <nil>, %v", res, err, want)
 			}
 			return nil
diff --git a/xds/internal/balancer/lrs/lrs_test.go b/xds/internal/balancer/lrs/lrs_test.go
index 4f2392fd..e79490c2 100644
--- a/xds/internal/balancer/lrs/lrs_test.go
+++ b/xds/internal/balancer/lrs/lrs_test.go
@@ -90,7 +90,7 @@ func equalClusterStats(a, b []*endpointpb.ClusterStats) bool {
 			s.LoadReportInterval = nil
 		}
 	}
-	return reflect.DeepEqual(a, b)
+	return cmp.Equal(a, b, cmp.Comparer(proto.Equal))
 }
 
 func Test_lrsStore_buildStats_drops(t *testing.T) {