From 19c3efee9df04c0f7f2ad3eca82a818d7c45b64b Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Mon, 31 Jul 2023 12:51:38 -0700 Subject: [PATCH] testutils: remove TestSubConns for future extensibility --- internal/testutils/balancer.go | 23 ++++------------ xds/internal/balancer/ringhash/picker_test.go | 26 ++++++++++++------- xds/internal/testutils/balancer_test.go | 6 ++--- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/internal/testutils/balancer.go b/internal/testutils/balancer.go index 3ef4bfb97bd6..babce5f19497 100644 --- a/internal/testutils/balancer.go +++ b/internal/testutils/balancer.go @@ -29,28 +29,12 @@ import ( "google.golang.org/grpc/resolver" ) -// TestSubConnsCount is the number of TestSubConns initialized as part of -// package init. -const TestSubConnsCount = 16 - // testingLogger wraps the logging methods from testing.T. type testingLogger interface { Log(args ...interface{}) Logf(format string, args ...interface{}) } -// TestSubConns contains a list of SubConns to be used in tests. -var TestSubConns []*TestSubConn - -func init() { - for i := 0; i < TestSubConnsCount; i++ { - TestSubConns = append(TestSubConns, &TestSubConn{ - id: fmt.Sprintf("sc%d", i), - ConnectCh: make(chan struct{}, 1), - }) - } -} - // TestSubConn implements the SubConn interface, to be used in tests. type TestSubConn struct { id string @@ -121,8 +105,11 @@ func NewTestClientConn(t *testing.T) *TestClientConn { // NewSubConn creates a new SubConn. func (tcc *TestClientConn) NewSubConn(a []resolver.Address, o balancer.NewSubConnOptions) (balancer.SubConn, error) { - sc := TestSubConns[tcc.subConnIdx] - sc.stateListener = o.StateListener + sc := &TestSubConn{ + id: fmt.Sprintf("sc%d", tcc.subConnIdx), + ConnectCh: make(chan struct{}, 1), + stateListener: o.StateListener, + } tcc.subConnIdx++ tcc.logger.Logf("testClientConn: NewSubConn(%v, %+v) => %s", a, o, sc) select { diff --git a/xds/internal/balancer/ringhash/picker_test.go b/xds/internal/balancer/ringhash/picker_test.go index 7accb1b4c00f..811ae8b683b2 100644 --- a/xds/internal/balancer/ringhash/picker_test.go +++ b/xds/internal/balancer/ringhash/picker_test.go @@ -31,10 +31,18 @@ import ( "google.golang.org/grpc/internal/testutils" ) +var testSubConns []*testutils.TestSubConn + +func init() { + for i := 0; i < 8; i++ { + testSubConns = append(testSubConns, &testutils.TestSubConn{ConnectCh: make(chan struct{}, 1)}) + } +} + func newTestRing(cStats []connectivity.State) *ring { var items []*ringEntry for i, st := range cStats { - testSC := testutils.TestSubConns[i] + testSC := testSubConns[i] items = append(items, &ringEntry{ idx: i, hash: uint64((i + 1) * 10), @@ -61,7 +69,7 @@ func (s) TestPickerPickFirstTwo(t *testing.T) { name: "picked is Ready", ring: newTestRing([]connectivity.State{connectivity.Ready, connectivity.Idle}), hash: 5, - wantSC: testutils.TestSubConns[0], + wantSC: testSubConns[0], }, { name: "picked is connecting, queue", @@ -74,13 +82,13 @@ func (s) TestPickerPickFirstTwo(t *testing.T) { ring: newTestRing([]connectivity.State{connectivity.Idle, connectivity.Idle}), hash: 5, wantErr: balancer.ErrNoSubConnAvailable, - wantSCToConnect: testutils.TestSubConns[0], + wantSCToConnect: testSubConns[0], }, { name: "picked is TransientFailure, next is ready, return", ring: newTestRing([]connectivity.State{connectivity.TransientFailure, connectivity.Ready}), hash: 5, - wantSC: testutils.TestSubConns[1], + wantSC: testSubConns[1], }, { name: "picked is TransientFailure, next is connecting, queue", @@ -93,7 +101,7 @@ func (s) TestPickerPickFirstTwo(t *testing.T) { ring: newTestRing([]connectivity.State{connectivity.TransientFailure, connectivity.Idle}), hash: 5, wantErr: balancer.ErrNoSubConnAvailable, - wantSCToConnect: testutils.TestSubConns[1], + wantSCToConnect: testSubConns[1], }, } for _, tt := range tests { @@ -106,7 +114,7 @@ func (s) TestPickerPickFirstTwo(t *testing.T) { t.Errorf("Pick() error = %v, wantErr %v", err, tt.wantErr) return } - if !cmp.Equal(got, balancer.PickResult{SubConn: tt.wantSC}, cmpOpts) { + if got.SubConn != tt.wantSC { t.Errorf("Pick() got = %v, want picked SubConn: %v", got, tt.wantSC) } if sc := tt.wantSCToConnect; sc != nil { @@ -163,7 +171,7 @@ func (s) TestPickerPickTriggerTFReturnReady(t *testing.T) { if err != nil { t.Fatalf("Pick() error = %v, want nil", err) } - if wantSC := testutils.TestSubConns[3]; pr.SubConn != wantSC { + if wantSC := testSubConns[3]; pr.SubConn != wantSC { t.Fatalf("Pick() = %v, want %v", pr.SubConn, wantSC) } // The first 3 SubConns, all in TransientFailure, should be queued to @@ -199,9 +207,9 @@ func (s) TestPickerPickTriggerTFWithIdle(t *testing.T) { } // SubConn 3 was in Idle, so should Connect() select { - case <-testutils.TestSubConns[2].ConnectCh: + case <-testSubConns[2].ConnectCh: case <-time.After(defaultTestShortTimeout): - t.Errorf("timeout waiting for Connect() from SubConn %v", testutils.TestSubConns[2]) + t.Errorf("timeout waiting for Connect() from SubConn %v", testSubConns[2]) } // The other SubConns, after the first Idle, should not be queued to // connect. diff --git a/xds/internal/testutils/balancer_test.go b/xds/internal/testutils/balancer_test.go index b5f7f665396c..73e0d1c12433 100644 --- a/xds/internal/testutils/balancer_test.go +++ b/xds/internal/testutils/balancer_test.go @@ -27,9 +27,9 @@ import ( func TestIsRoundRobin(t *testing.T) { var ( - sc1 = testutils.TestSubConns[0] - sc2 = testutils.TestSubConns[1] - sc3 = testutils.TestSubConns[2] + sc1 = &testutils.TestSubConn{} + sc2 = &testutils.TestSubConn{} + sc3 = &testutils.TestSubConn{} ) testCases := []struct {