[1.14.x] prototest: fix early return condition in AssertElementsMatch

agent/grpc-external/services/peerstream/stream_test.go

@@ -547,7 +547,7 @@ func TestStreamResources_Server_StreamTracker(t *testing.T) {
 	it := incrementalTime{
 		base: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
 	}
-	waitUntil := it.FutureNow(6)
+	waitUntil := it.FutureNow(7)
 
 	srv, store := newTestServer(t, nil)
 	srv.Tracker.setClock(it.Now)
@@ -1439,7 +1439,9 @@ func makeClient(t *testing.T, srv *testServer, peerID string) *MockClient {
 	// Note that server address may not come as an initial message
 	for _, resourceURL := range []string{
 		pbpeerstream.TypeURLExportedService,
+		pbpeerstream.TypeURLExportedServiceList,
 		pbpeerstream.TypeURLPeeringTrustBundle,
+		// only dialers request, which is why this is absent below
 		pbpeerstream.TypeURLPeeringServerAddresses,
 	} {
 		init := &pbpeerstream.ReplicationMessage{
@@ -1468,7 +1470,7 @@ func makeClient(t *testing.T, srv *testServer, peerID string) *MockClient {
 		{
 			Payload: &pbpeerstream.ReplicationMessage_Request_{
 				Request: &pbpeerstream.ReplicationMessage_Request{
-					ResourceURL: pbpeerstream.TypeURLPeeringTrustBundle,
+					ResourceURL: pbpeerstream.TypeURLExportedServiceList,
 					// The PeerID field is only set for the messages coming FROM
 					// the establishing side and are going to be empty from the
 					// other side.
@@ -1479,7 +1481,7 @@ func makeClient(t *testing.T, srv *testServer, peerID string) *MockClient {
 		{
 			Payload: &pbpeerstream.ReplicationMessage_Request_{
 				Request: &pbpeerstream.ReplicationMessage_Request{
-					ResourceURL: pbpeerstream.TypeURLPeeringServerAddresses,
+					ResourceURL: pbpeerstream.TypeURLPeeringTrustBundle,
 					// The PeerID field is only set for the messages coming FROM
 					// the establishing side and are going to be empty from the
 					// other side.

proto/prototest/testing.go

@@ -26,14 +26,25 @@ func AssertDeepEqual(t testing.TB, x, y interface{}, opts ...cmp.Option) {
 func AssertElementsMatch[V any](
 	t testing.TB, listX, listY []V, opts ...cmp.Option,
 ) {
-	t.Helper()
+	diff := diffElements(listX, listY, opts...)
+	if diff != "" {
+		t.Fatalf("assertion failed: slices do not have matching elements\n--- expected\n+++ actual\n%v", diff)
+	}
+}
 
+func diffElements[V any](
+	listX, listY []V, opts ...cmp.Option,
+) string {
 	if len(listX) == 0 && len(listY) == 0 {
-		return
+		return ""
 	}
 
 	opts = append(opts, protocmp.Transform())
 
+	if len(listX) != len(listY) {
+		return cmp.Diff(listX, listY, opts...)
+	}
+
 	// dump into a map keyed by sliceID
 	mapX := make(map[int]V)
 	for i, val := range listX {
@@ -57,8 +68,8 @@ func AssertElementsMatch[V any](
 		}
 	}
 
-	if len(outX) == len(outY) && len(listX) == len(listY) {
-		return // matches
+	if len(outX) == len(listX) && len(outY) == len(listY) {
+		return "" // matches
 	}
 
 	// dump remainder into the slice so we can generate a useful error
@@ -69,7 +80,5 @@ func AssertElementsMatch[V any](
 		outY = append(outY, itemY)
 	}
 
-	if diff := cmp.Diff(outX, outY, opts...); diff != "" {
-		t.Fatalf("assertion failed: slices do not have matching elements\n--- expected\n+++ actual\n%v", diff)
-	}
+	return cmp.Diff(outX, outY, opts...)
 }

proto/prototest/testing_test.go

@@ -0,0 +1,103 @@
+package prototest
+
+import (
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+type wrap struct {
+	V int
+	O string
+}
+
+func (w *wrap) String() string {
+	return strconv.Itoa(w.V)
+}
+
+func (w *wrap) GoString() string {
+	return w.String()
+}
+
+func TestDiffElements_noProtobufs(t *testing.T) {
+	// NOTE: this test only tests non-protobuf slices initially
+
+	type testcase struct {
+		a, b    []*wrap
+		notSame bool
+	}
+
+	run := func(t *testing.T, tc testcase) {
+		diff := diffElements(tc.a, tc.b)
+		if tc.notSame {
+			require.False(t, diff == "", "expected not to be the same")
+		} else {
+			require.True(t, diff == "", "expected to be the same")
+		}
+	}
+
+	w := func(v int) *wrap {
+		return &wrap{V: v}
+	}
+
+	cases := map[string]testcase{
+		"nil":           {},
+		"empty":         {a: []*wrap{}, b: []*wrap{}},
+		"nil and empty": {a: []*wrap{}, b: nil},
+		"ordered match": {
+			a: []*wrap{w(1), w(22), w(303), w(43004), w(-5)},
+			b: []*wrap{w(1), w(22), w(303), w(43004), w(-5)},
+		},
+		"permuted match": {
+			a: []*wrap{w(1), w(22), w(303), w(43004), w(-5)},
+			b: []*wrap{w(-5), w(43004), w(303), w(22), w(1)},
+		},
+		"duplicates": {
+			a: []*wrap{w(1), w(2), w(2), w(3)},
+			b: []*wrap{w(2), w(1), w(3), w(2)},
+		},
+		// no match
+		"1 vs nil": {
+			a:       []*wrap{w(1)},
+			b:       nil,
+			notSame: true,
+		},
+		"1 vs 2": {
+			a:       []*wrap{w(1)},
+			b:       []*wrap{w(2)},
+			notSame: true,
+		},
+		"1,2 vs 2,3": {
+			a:       []*wrap{w(1), w(2)},
+			b:       []*wrap{w(2), w(3)},
+			notSame: true,
+		},
+		"1,2 vs 1,2,3": {
+			a:       []*wrap{w(1), w(2)},
+			b:       []*wrap{w(1), w(2), w(3)},
+			notSame: true,
+		},
+		"duplicates omitted": {
+			a:       []*wrap{w(1), w(2), w(2), w(3)},
+			b:       []*wrap{w(1), w(3), w(2)},
+			notSame: true,
+		},
+	}
+
+	allCases := make(map[string]testcase)
+	for name, tc := range cases {
+		allCases[name] = tc
+		allCases[name+" (flipped)"] = testcase{
+			a:       tc.b,
+			b:       tc.a,
+			notSame: tc.notSame,
+		}
+	}
+
+	for name, tc := range allCases {
+		t.Run(name, func(t *testing.T) {
+			run(t, tc)
+		})
+	}
+}