Skip to content

Commit dad518a

Browse files
easwarseaswars
authored
xds: Refactor/cleanup xds client tests. (#3920)
1 parent b2c5f4a commit dad518a

7 files changed

+782
-709
lines changed

‎xds/internal/client/client_cds_test.go

+29-10
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,14 @@ import (
3232
"google.golang.org/grpc/xds/internal/version"
3333
)
3434

35-
func (s) TestValidateCluster(t *testing.T) {
36-
const (
37-
clusterName = "clusterName"
38-
serviceName = "service"
39-
)
40-
var (
41-
emptyUpdate = ClusterUpdate{ServiceName: "", EnableLRS: false}
42-
)
35+
const (
36+
clusterName = "clusterName"
37+
serviceName = "service"
38+
)
39+
40+
var emptyUpdate = ClusterUpdate{ServiceName: "", EnableLRS: false}
4341

42+
func (s) TestValidateCluster_Failure(t *testing.T) {
4443
tests := []struct {
4544
name string
4645
cluster *v3clusterpb.Cluster
@@ -98,6 +97,23 @@ func (s) TestValidateCluster(t *testing.T) {
9897
wantUpdate: emptyUpdate,
9998
wantErr: true,
10099
},
100+
}
101+
102+
for _, test := range tests {
103+
t.Run(test.name, func(t *testing.T) {
104+
if update, err := validateCluster(test.cluster); err == nil {
105+
t.Errorf("validateCluster(%+v) = %v, wanted error", test.cluster, update)
106+
}
107+
})
108+
}
109+
}
110+
111+
func (s) TestValidateCluster_Success(t *testing.T) {
112+
tests := []struct {
113+
name string
114+
cluster *v3clusterpb.Cluster
115+
wantUpdate ClusterUpdate
116+
}{
101117
{
102118
name: "happy-case-no-service-name-no-lrs",
103119
cluster: &v3clusterpb.Cluster{
@@ -156,8 +172,11 @@ func (s) TestValidateCluster(t *testing.T) {
156172
for _, test := range tests {
157173
t.Run(test.name, func(t *testing.T) {
158174
update, err := validateCluster(test.cluster)
159-
if ((err != nil) != test.wantErr) || !cmp.Equal(update, test.wantUpdate, cmpopts.EquateEmpty()) {
160-
t.Errorf("validateCluster(%+v) = (%v, %v), wantErr: (%v, %v)", test.cluster, update, err, test.wantUpdate, test.wantErr)
175+
if err != nil {
176+
t.Errorf("validateCluster(%+v) failed: %v", test.cluster, err)
177+
}
178+
if !cmp.Equal(update, test.wantUpdate, cmpopts.EquateEmpty()) {
179+
t.Errorf("validateCluster(%+v) = %v, want: %v", test.cluster, update, test.wantUpdate)
161180
}
162181
})
163182
}

‎xds/internal/client/client_test.go

+90-33
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,13 @@ package client
2020

2121
import (
2222
"context"
23+
"fmt"
2324
"testing"
2425
"time"
2526

27+
"github.com/google/go-cmp/cmp"
28+
"github.com/google/go-cmp/cmp/cmpopts"
29+
2630
"google.golang.org/grpc"
2731
"google.golang.org/grpc/internal/grpctest"
2832
"google.golang.org/grpc/internal/testutils"
@@ -49,7 +53,8 @@ const (
4953
testEDSName = "test-eds"
5054

5155
defaultTestWatchExpiryTimeout = 500 * time.Millisecond
52-
defaultTestTimeout = 1 * time.Second
56+
defaultTestTimeout = 5 * time.Second
57+
defaultTestShortTimeout = 10 * time.Millisecond // For events expected to *not* happen.
5358
)
5459

5560
func clientOpts(balancerName string, overrideWatchExpiryTImeout bool) Options {
@@ -68,24 +73,22 @@ func clientOpts(balancerName string, overrideWatchExpiryTImeout bool) Options {
6873
}
6974

7075
type testAPIClient struct {
71-
r UpdateHandler
72-
7376
addWatches map[ResourceType]*testutils.Channel
7477
removeWatches map[ResourceType]*testutils.Channel
7578
}
7679

77-
func overrideNewAPIClient() (<-chan *testAPIClient, func()) {
80+
func overrideNewAPIClient() (*testutils.Channel, func()) {
7881
origNewAPIClient := newAPIClient
79-
ch := make(chan *testAPIClient, 1)
82+
ch := testutils.NewChannel()
8083
newAPIClient = func(apiVersion version.TransportAPI, cc *grpc.ClientConn, opts BuildOptions) (APIClient, error) {
81-
ret := newTestAPIClient(opts.Parent)
82-
ch <- ret
84+
ret := newTestAPIClient()
85+
ch.Send(ret)
8386
return ret, nil
8487
}
8588
return ch, func() { newAPIClient = origNewAPIClient }
8689
}
8790

88-
func newTestAPIClient(r UpdateHandler) *testAPIClient {
91+
func newTestAPIClient() *testAPIClient {
8992
addWatches := map[ResourceType]*testutils.Channel{
9093
ListenerResource: testutils.NewChannel(),
9194
RouteConfigResource: testutils.NewChannel(),
@@ -99,7 +102,6 @@ func newTestAPIClient(r UpdateHandler) *testAPIClient {
99102
EndpointsResource: testutils.NewChannel(),
100103
}
101104
return &testAPIClient{
102-
r: r,
103105
addWatches: addWatches,
104106
removeWatches: removeWatches,
105107
}
@@ -121,53 +123,108 @@ func (c *testAPIClient) Close() {}
121123
// TestWatchCallAnotherWatch covers the case where watch() is called inline by a
122124
// callback. It makes sure it doesn't cause a deadlock.
123125
func (s) TestWatchCallAnotherWatch(t *testing.T) {
124-
v2ClientCh, cleanup := overrideNewAPIClient()
126+
apiClientCh, cleanup := overrideNewAPIClient()
125127
defer cleanup()
126128

127-
c, err := New(clientOpts(testXDSServer, false))
129+
client, err := New(clientOpts(testXDSServer, false))
128130
if err != nil {
129131
t.Fatalf("failed to create client: %v", err)
130132
}
131-
defer c.Close()
133+
defer client.Close()
132134

133-
v2Client := <-v2ClientCh
135+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
136+
defer cancel()
137+
c, err := apiClientCh.Receive(ctx)
138+
if err != nil {
139+
t.Fatalf("timeout when waiting for API client to be created: %v", err)
140+
}
141+
apiClient := c.(*testAPIClient)
134142

135143
clusterUpdateCh := testutils.NewChannel()
136144
firstTime := true
137-
c.WatchCluster(testCDSName, func(update ClusterUpdate, err error) {
145+
client.WatchCluster(testCDSName, func(update ClusterUpdate, err error) {
138146
clusterUpdateCh.Send(clusterUpdateErr{u: update, err: err})
139147
// Calls another watch inline, to ensure there's deadlock.
140-
c.WatchCluster("another-random-name", func(ClusterUpdate, error) {})
148+
client.WatchCluster("another-random-name", func(ClusterUpdate, error) {})
141149

142-
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
143-
defer cancel()
144-
if _, err := v2Client.addWatches[ClusterResource].Receive(ctx); firstTime && err != nil {
150+
if _, err := apiClient.addWatches[ClusterResource].Receive(ctx); firstTime && err != nil {
145151
t.Fatalf("want new watch to start, got error %v", err)
146152
}
147153
firstTime = false
148154
})
149-
150-
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
151-
defer cancel()
152-
if _, err := v2Client.addWatches[ClusterResource].Receive(ctx); err != nil {
155+
if _, err := apiClient.addWatches[ClusterResource].Receive(ctx); err != nil {
153156
t.Fatalf("want new watch to start, got error %v", err)
154157
}
155158

156159
wantUpdate := ClusterUpdate{ServiceName: testEDSName}
157-
v2Client.r.NewClusters(map[string]ClusterUpdate{
158-
testCDSName: wantUpdate,
159-
})
160-
161-
if u, err := clusterUpdateCh.Receive(ctx); err != nil || u != (clusterUpdateErr{wantUpdate, nil}) {
162-
t.Errorf("unexpected clusterUpdate: %v, error receiving from channel: %v", u, err)
160+
client.NewClusters(map[string]ClusterUpdate{testCDSName: wantUpdate})
161+
if err := verifyClusterUpdate(ctx, clusterUpdateCh, wantUpdate); err != nil {
162+
t.Fatal(err)
163163
}
164164

165165
wantUpdate2 := ClusterUpdate{ServiceName: testEDSName + "2"}
166-
v2Client.r.NewClusters(map[string]ClusterUpdate{
167-
testCDSName: wantUpdate2,
168-
})
166+
client.NewClusters(map[string]ClusterUpdate{testCDSName: wantUpdate2})
167+
if err := verifyClusterUpdate(ctx, clusterUpdateCh, wantUpdate2); err != nil {
168+
t.Fatal(err)
169+
}
170+
}
171+
172+
func verifyListenerUpdate(ctx context.Context, updateCh *testutils.Channel, wantUpdate ListenerUpdate) error {
173+
u, err := updateCh.Receive(ctx)
174+
if err != nil {
175+
return fmt.Errorf("timeout when waiting for listener update: %v", err)
176+
}
177+
gotUpdate := u.(ldsUpdateErr)
178+
if gotUpdate.err != nil || !cmp.Equal(gotUpdate.u, wantUpdate) {
179+
return fmt.Errorf("unexpected endpointsUpdate: (%v, %v), want: (%v, nil)", gotUpdate.u, gotUpdate.err, wantUpdate)
180+
}
181+
return nil
182+
}
183+
184+
func verifyRouteConfigUpdate(ctx context.Context, updateCh *testutils.Channel, wantUpdate RouteConfigUpdate) error {
185+
u, err := updateCh.Receive(ctx)
186+
if err != nil {
187+
return fmt.Errorf("timeout when waiting for route configuration update: %v", err)
188+
}
189+
gotUpdate := u.(rdsUpdateErr)
190+
if gotUpdate.err != nil || !cmp.Equal(gotUpdate.u, wantUpdate) {
191+
return fmt.Errorf("unexpected route config update: (%v, %v), want: (%v, nil)", gotUpdate.u, gotUpdate.err, wantUpdate)
192+
}
193+
return nil
194+
}
195+
196+
func verifyServiceUpdate(ctx context.Context, updateCh *testutils.Channel, wantUpdate ServiceUpdate) error {
197+
u, err := updateCh.Receive(ctx)
198+
if err != nil {
199+
return fmt.Errorf("timeout when waiting for service update: %v", err)
200+
}
201+
gotUpdate := u.(serviceUpdateErr)
202+
if gotUpdate.err != nil || !cmp.Equal(gotUpdate.u, wantUpdate, cmpopts.EquateEmpty()) {
203+
return fmt.Errorf("unexpected service update: (%v, %v), want: (%v, nil)", gotUpdate.u, gotUpdate.err, wantUpdate)
204+
}
205+
return nil
206+
}
169207

170-
if u, err := clusterUpdateCh.Receive(ctx); err != nil || u != (clusterUpdateErr{wantUpdate2, nil}) {
171-
t.Errorf("unexpected clusterUpdate: %v, error receiving from channel: %v", u, err)
208+
func verifyClusterUpdate(ctx context.Context, updateCh *testutils.Channel, wantUpdate ClusterUpdate) error {
209+
u, err := updateCh.Receive(ctx)
210+
if err != nil {
211+
return fmt.Errorf("timeout when waiting for cluster update: %v", err)
212+
}
213+
gotUpdate := u.(clusterUpdateErr)
214+
if gotUpdate.err != nil || !cmp.Equal(gotUpdate.u, wantUpdate) {
215+
return fmt.Errorf("unexpected clusterUpdate: (%v, %v), want: (%v, nil)", gotUpdate.u, gotUpdate.err, wantUpdate)
216+
}
217+
return nil
218+
}
219+
220+
func verifyEndpointsUpdate(ctx context.Context, updateCh *testutils.Channel, wantUpdate EndpointsUpdate) error {
221+
u, err := updateCh.Receive(ctx)
222+
if err != nil {
223+
return fmt.Errorf("timeout when waiting for endpoints update: %v", err)
224+
}
225+
gotUpdate := u.(endpointsUpdateErr)
226+
if gotUpdate.err != nil || !cmp.Equal(gotUpdate.u, wantUpdate, cmpopts.EquateEmpty()) {
227+
return fmt.Errorf("unexpected endpointsUpdate: (%v, %v), want: (%v, nil)", gotUpdate.u, gotUpdate.err, wantUpdate)
172228
}
229+
return nil
173230
}

0 commit comments

Comments
 (0)