Skip to content

Commit

Permalink
[federation_target_parsing] refactor resolver tests to check for close
Browse files Browse the repository at this point in the history
  • Loading branch information
@menghanl
menghanl committed Nov 3, 2021
1 parent f8ea440 commit 40ab629
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 68 deletions.
104 changes: 38 additions & 66 deletions xds/internal/resolver/xds_resolver_test.go
View file
Expand Up @@ -228,16 +228,29 @@ func (s) TestResolverBuilder_xdsCredsBootstrapMismatch(t *testing.T) {
}

type setupOpts struct {
xdsClientFunc func() (xdsclient.XDSClient, error)
target *resolver.Target
bootstrapC *bootstrap.Config
target *resolver.Target
}

func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *testClientConn, func()) {
func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *fakeclient.Client, *testClientConn, func()) {
t.Helper()

fc := fakeclient.NewClient()
if opts.bootstrapC != nil {
fc.SetBootstrapConfig(opts.bootstrapC)
}
oldClientMaker := newXDSClient
newXDSClient = opts.xdsClientFunc
newXDSClient = func() (xdsclient.XDSClient, error) {
return fc, nil
}
cancel := func() {
// Make sure the xDS client is closed, in all (successful or failed)
// cases.
select {
case <-time.After(defaultTestTimeout):
t.Fatalf("timeout waiting for close")
case <-fc.Closed.Done():
}
newXDSClient = oldClientMaker
}

Expand All @@ -255,7 +268,10 @@ func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *testClientConn, fun
if err != nil {
t.Fatalf("builder.Build(%v) returned err: %v", target, err)
}
return r.(*xdsResolver), tcc, cancel
return r.(*xdsResolver), fc, tcc, func() {
r.Close()
cancel()
}
}

// waitForWatchListener waits for the WatchListener method to be called on the
Expand Down Expand Up @@ -361,11 +377,9 @@ func (s) TestXDSResolverResourceNameToWatch(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsC.SetBootstrapConfig(tt.bc)
xdsR, _, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
target: tt.target,
xdsR, xdsC, _, cancel := testSetup(t, setupOpts{
bootstrapC: tt.bc,
target: tt.target,
})
defer cancel()
defer xdsR.Close()
Expand All @@ -380,10 +394,7 @@ func (s) TestXDSResolverResourceNameToWatch(t *testing.T) {
// TestXDSResolverWatchCallbackAfterClose tests the case where a service update
// from the underlying xdsClient is received after the resolver is closed.
func (s) TestXDSResolverWatchCallbackAfterClose(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer cancel()

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
Expand Down Expand Up @@ -412,10 +423,7 @@ func (s) TestXDSResolverWatchCallbackAfterClose(t *testing.T) {
// TestXDSResolverCloseClosesXDSClient tests that the XDS resolver's Close
// method closes the XDS client.
func (s) TestXDSResolverCloseClosesXDSClient(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, _, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, _, cancel := testSetup(t, setupOpts{})
defer cancel()
xdsR.Close()
if !xdsC.Closed.HasFired() {
Expand All @@ -426,10 +434,7 @@ func (s) TestXDSResolverCloseClosesXDSClient(t *testing.T) {
// TestXDSResolverBadServiceUpdate tests the case the xdsClient returns a bad
// service update.
func (s) TestXDSResolverBadServiceUpdate(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

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

Expand Down Expand Up @@ -591,10 +593,7 @@ func (s) TestXDSResolverRequestHash(t *testing.T) {
env.RingHashSupport = true
defer func() { env.RingHashSupport = oldRH }()

xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down Expand Up @@ -651,10 +650,7 @@ func (s) TestXDSResolverRequestHash(t *testing.T) {
// TestXDSResolverRemovedWithRPCs tests the case where a config selector sends
// an empty update to the resolver after the resource is removed.
func (s) TestXDSResolverRemovedWithRPCs(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer cancel()
defer xdsR.Close()

Expand Down Expand Up @@ -711,10 +707,7 @@ func (s) TestXDSResolverRemovedWithRPCs(t *testing.T) {
// TestXDSResolverRemovedResource tests for proper behavior after a resource is
// removed.
func (s) TestXDSResolverRemovedResource(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer cancel()
defer xdsR.Close()

Expand Down Expand Up @@ -819,10 +812,7 @@ func (s) TestXDSResolverRemovedResource(t *testing.T) {
}

func (s) TestXDSResolverWRR(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down Expand Up @@ -879,10 +869,7 @@ func (s) TestXDSResolverWRR(t *testing.T) {
}

func (s) TestXDSResolverMaxStreamDuration(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down Expand Up @@ -972,10 +959,7 @@ func (s) TestXDSResolverMaxStreamDuration(t *testing.T) {
// TestXDSResolverDelayedOnCommitted tests that clusters remain in service
// config if RPCs are in flight.
func (s) TestXDSResolverDelayedOnCommitted(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down Expand Up @@ -1121,10 +1105,7 @@ func (s) TestXDSResolverDelayedOnCommitted(t *testing.T) {
// TestXDSResolverUpdates tests the cases where the resolver gets a good update
// after an error, and an error after the good update.
func (s) TestXDSResolverGoodUpdateAfterError(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down Expand Up @@ -1175,10 +1156,7 @@ func (s) TestXDSResolverGoodUpdateAfterError(t *testing.T) {
// a ResourceNotFoundError. It should generate a service config picking
// weighted_target, but no child balancers.
func (s) TestXDSResolverResourceNotFoundError(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down Expand Up @@ -1221,10 +1199,7 @@ func (s) TestXDSResolverResourceNotFoundError(t *testing.T) {
//
// This test case also makes sure the resolver doesn't panic.
func (s) TestXDSResolverMultipleLDSUpdates(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down Expand Up @@ -1396,10 +1371,7 @@ func (s) TestXDSResolverHTTPFilters(t *testing.T) {

for i, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil },
})
xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{})
defer xdsR.Close()
defer cancel()

Expand Down
4 changes: 2 additions & 2 deletions xds/internal/testutils/fakeclient/client.go
View file
Expand Up @@ -305,11 +305,11 @@ func NewClient() *Client {
func NewClientWithName(name string) *Client {
return &Client{
name: name,
ldsWatchCh: testutils.NewChannel(),
ldsWatchCh: testutils.NewChannelWithSize(10),
rdsWatchCh: testutils.NewChannelWithSize(10),
cdsWatchCh: testutils.NewChannelWithSize(10),
edsWatchCh: testutils.NewChannelWithSize(10),
ldsCancelCh: testutils.NewChannel(),
ldsCancelCh: testutils.NewChannelWithSize(10),
rdsCancelCh: testutils.NewChannelWithSize(10),
cdsCancelCh: testutils.NewChannelWithSize(10),
edsCancelCh: testutils.NewChannelWithSize(10),
Expand Down

0 comments on commit 40ab629

Please sign in to comment.