testutils: remove TestSubConns for future extensibility (grpc#6492)

dfawley · Aug 1, 2023 · 0246373 · 0246373
1 parent c635404
commit 0246373
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 30 deletions.
diff --git 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
@@ -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
@@ -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 {