Skip to content

Commit 53a360b

Browse files
menghanlmenghanl
committed
[federation_target_parsing] refactor resolver tests to check for close
1 parent f8ea440 commit 53a360b

File tree

2 files changed

+40
-66
lines changed

2 files changed

+40
-66
lines changed

‎xds/internal/resolver/xds_resolver_test.go‎

Lines changed: 38 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -228,16 +228,29 @@ func (s) TestResolverBuilder_xdsCredsBootstrapMismatch(t *testing.T) {
228228
}
229229

230230
type setupOpts struct {
231-
xdsClientFunc func() (xdsclient.XDSClient, error)
232-
target *resolver.Target
231+
bootstrapC *bootstrap.Config
232+
target *resolver.Target
233233
}
234234

235-
func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *testClientConn, func()) {
235+
func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *fakeclient.Client, *testClientConn, func()) {
236236
t.Helper()
237237

238+
fc := fakeclient.NewClient()
239+
if opts.bootstrapC != nil {
240+
fc.SetBootstrapConfig(opts.bootstrapC)
241+
}
238242
oldClientMaker := newXDSClient
239-
newXDSClient = opts.xdsClientFunc
243+
newXDSClient = func() (xdsclient.XDSClient, error) {
244+
return fc, nil
245+
}
240246
cancel := func() {
247+
// Make sure the xDS client is closed, in all (successful or failed)
248+
// cases.
249+
select {
250+
case <-time.After(defaultTestTimeout):
251+
t.Fatalf("timeout waiting for close")
252+
case <-fc.Closed.Done():
253+
}
241254
newXDSClient = oldClientMaker
242255
}
243256

@@ -255,7 +268,10 @@ func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *testClientConn, fun
255268
if err != nil {
256269
t.Fatalf("builder.Build(%v) returned err: %v", target, err)
257270
}
258-
return r.(*xdsResolver), tcc, cancel
271+
return r.(*xdsResolver), fc, tcc, func() {
272+
r.Close()
273+
cancel()
274+
}
259275
}
260276

261277
// waitForWatchListener waits for the WatchListener method to be called on the
@@ -361,11 +377,9 @@ func (s) TestXDSResolverResourceNameToWatch(t *testing.T) {
361377
}
362378
for _, tt := range tests {
363379
t.Run(tt.name, func(t *testing.T) {
364-
xdsC := fakeclient.NewClient()
365-
xdsC.SetBootstrapConfig(tt.bc)
366-
xdsR, _, cancel := testSetup(t, setupOpts{
367-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
368-
target: tt.target,
380+
xdsR, xdsC, _, cancel := testSetup(t, setupOpts{
381+
bootstrapC: tt.bc,
382+
target: tt.target,
369383
})
370384
defer cancel()
371385
defer xdsR.Close()
@@ -380,10 +394,7 @@ func (s) TestXDSResolverResourceNameToWatch(t *testing.T) {
380394
// TestXDSResolverWatchCallbackAfterClose tests the case where a service update
381395
// from the underlying xdsClient is received after the resolver is closed.
382396
func (s) TestXDSResolverWatchCallbackAfterClose(t *testing.T) {
383-
xdsC := fakeclient.NewClient()
384-
xdsR, tcc, cancel := testSetup(t, setupOpts{
385-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
386-
})
397+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
387398
defer cancel()
388399

389400
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
@@ -413,9 +424,7 @@ func (s) TestXDSResolverWatchCallbackAfterClose(t *testing.T) {
413424
// method closes the XDS client.
414425
func (s) TestXDSResolverCloseClosesXDSClient(t *testing.T) {
415426
xdsC := fakeclient.NewClient()
416-
xdsR, _, cancel := testSetup(t, setupOpts{
417-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
418-
})
427+
xdsR, xdsC, _, cancel := testSetup(t, setupOpts{})
419428
defer cancel()
420429
xdsR.Close()
421430
if !xdsC.Closed.HasFired() {
@@ -427,9 +436,7 @@ func (s) TestXDSResolverCloseClosesXDSClient(t *testing.T) {
427436
// service update.
428437
func (s) TestXDSResolverBadServiceUpdate(t *testing.T) {
429438
xdsC := fakeclient.NewClient()
430-
xdsR, tcc, cancel := testSetup(t, setupOpts{
431-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
432-
})
439+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
433440
defer xdsR.Close()
434441
defer cancel()
435442

@@ -452,10 +459,7 @@ func (s) TestXDSResolverBadServiceUpdate(t *testing.T) {
452459
// TestXDSResolverGoodServiceUpdate tests the happy case where the resolver
453460
// gets a good service update from the xdsClient.
454461
func (s) TestXDSResolverGoodServiceUpdate(t *testing.T) {
455-
xdsC := fakeclient.NewClient()
456-
xdsR, tcc, cancel := testSetup(t, setupOpts{
457-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
458-
})
462+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
459463
defer xdsR.Close()
460464
defer cancel()
461465

@@ -591,10 +595,7 @@ func (s) TestXDSResolverRequestHash(t *testing.T) {
591595
env.RingHashSupport = true
592596
defer func() { env.RingHashSupport = oldRH }()
593597

594-
xdsC := fakeclient.NewClient()
595-
xdsR, tcc, cancel := testSetup(t, setupOpts{
596-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
597-
})
598+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
598599
defer xdsR.Close()
599600
defer cancel()
600601

@@ -651,10 +652,7 @@ func (s) TestXDSResolverRequestHash(t *testing.T) {
651652
// TestXDSResolverRemovedWithRPCs tests the case where a config selector sends
652653
// an empty update to the resolver after the resource is removed.
653654
func (s) TestXDSResolverRemovedWithRPCs(t *testing.T) {
654-
xdsC := fakeclient.NewClient()
655-
xdsR, tcc, cancel := testSetup(t, setupOpts{
656-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
657-
})
655+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
658656
defer cancel()
659657
defer xdsR.Close()
660658

@@ -711,10 +709,7 @@ func (s) TestXDSResolverRemovedWithRPCs(t *testing.T) {
711709
// TestXDSResolverRemovedResource tests for proper behavior after a resource is
712710
// removed.
713711
func (s) TestXDSResolverRemovedResource(t *testing.T) {
714-
xdsC := fakeclient.NewClient()
715-
xdsR, tcc, cancel := testSetup(t, setupOpts{
716-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
717-
})
712+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
718713
defer cancel()
719714
defer xdsR.Close()
720715

@@ -819,10 +814,7 @@ func (s) TestXDSResolverRemovedResource(t *testing.T) {
819814
}
820815

821816
func (s) TestXDSResolverWRR(t *testing.T) {
822-
xdsC := fakeclient.NewClient()
823-
xdsR, tcc, cancel := testSetup(t, setupOpts{
824-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
825-
})
817+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
826818
defer xdsR.Close()
827819
defer cancel()
828820

@@ -879,10 +871,7 @@ func (s) TestXDSResolverWRR(t *testing.T) {
879871
}
880872

881873
func (s) TestXDSResolverMaxStreamDuration(t *testing.T) {
882-
xdsC := fakeclient.NewClient()
883-
xdsR, tcc, cancel := testSetup(t, setupOpts{
884-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
885-
})
874+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
886875
defer xdsR.Close()
887876
defer cancel()
888877

@@ -972,10 +961,7 @@ func (s) TestXDSResolverMaxStreamDuration(t *testing.T) {
972961
// TestXDSResolverDelayedOnCommitted tests that clusters remain in service
973962
// config if RPCs are in flight.
974963
func (s) TestXDSResolverDelayedOnCommitted(t *testing.T) {
975-
xdsC := fakeclient.NewClient()
976-
xdsR, tcc, cancel := testSetup(t, setupOpts{
977-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
978-
})
964+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
979965
defer xdsR.Close()
980966
defer cancel()
981967

@@ -1121,10 +1107,7 @@ func (s) TestXDSResolverDelayedOnCommitted(t *testing.T) {
11211107
// TestXDSResolverUpdates tests the cases where the resolver gets a good update
11221108
// after an error, and an error after the good update.
11231109
func (s) TestXDSResolverGoodUpdateAfterError(t *testing.T) {
1124-
xdsC := fakeclient.NewClient()
1125-
xdsR, tcc, cancel := testSetup(t, setupOpts{
1126-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
1127-
})
1110+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
11281111
defer xdsR.Close()
11291112
defer cancel()
11301113

@@ -1175,10 +1158,7 @@ func (s) TestXDSResolverGoodUpdateAfterError(t *testing.T) {
11751158
// a ResourceNotFoundError. It should generate a service config picking
11761159
// weighted_target, but no child balancers.
11771160
func (s) TestXDSResolverResourceNotFoundError(t *testing.T) {
1178-
xdsC := fakeclient.NewClient()
1179-
xdsR, tcc, cancel := testSetup(t, setupOpts{
1180-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
1181-
})
1161+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
11821162
defer xdsR.Close()
11831163
defer cancel()
11841164

@@ -1221,10 +1201,7 @@ func (s) TestXDSResolverResourceNotFoundError(t *testing.T) {
12211201
//
12221202
// This test case also makes sure the resolver doesn't panic.
12231203
func (s) TestXDSResolverMultipleLDSUpdates(t *testing.T) {
1224-
xdsC := fakeclient.NewClient()
1225-
xdsR, tcc, cancel := testSetup(t, setupOpts{
1226-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
1227-
})
1204+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
12281205
defer xdsR.Close()
12291206
defer cancel()
12301207

@@ -1396,10 +1373,7 @@ func (s) TestXDSResolverHTTPFilters(t *testing.T) {
13961373

13971374
for i, tc := range testCases {
13981375
t.Run(tc.name, func(t *testing.T) {
1399-
xdsC := fakeclient.NewClient()
1400-
xdsR, tcc, cancel := testSetup(t, setupOpts{
1401-
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
1402-
})
1376+
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
14031377
defer xdsR.Close()
14041378
defer cancel()
14051379

‎xds/internal/testutils/fakeclient/client.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,11 @@ func NewClient() *Client {
305305
func NewClientWithName(name string) *Client {
306306
return &Client{
307307
name: name,
308-
ldsWatchCh: testutils.NewChannel(),
308+
ldsWatchCh: testutils.NewChannelWithSize(10),
309309
rdsWatchCh: testutils.NewChannelWithSize(10),
310310
cdsWatchCh: testutils.NewChannelWithSize(10),
311311
edsWatchCh: testutils.NewChannelWithSize(10),
312-
ldsCancelCh: testutils.NewChannel(),
312+
ldsCancelCh: testutils.NewChannelWithSize(10),
313313
rdsCancelCh: testutils.NewChannelWithSize(10),
314314
cdsCancelCh: testutils.NewChannelWithSize(10),
315315
edsCancelCh: testutils.NewChannelWithSize(10),

0 commit comments

Comments
 (0)