From ef292d2057cfdcfa8c07dc32e5b6a6974a2abdad Mon Sep 17 00:00:00 2001 From: hoo Date: Sun, 1 Jun 2025 00:38:25 +0900 Subject: [PATCH 01/13] add tests for `ExitIdle` --- internal/balancergroup/balancergroup_test.go | 82 +++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index 65e5af6312b8..25529b66ba0b 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -505,7 +505,7 @@ func (s) TestBalancerExitIdleOne(t *testing.T) { builder := balancer.Get(balancerName) bg.Add(testBalancerIDs[0], builder) - // Call ExitIdle on the child policy. + // Call ExitIdleOne on the child policy. bg.ExitIdleOne(testBalancerIDs[0]) select { case <-time.After(time.Second): @@ -640,3 +640,83 @@ func (s) TestBalancerGracefulSwitch(t *testing.T) { } } } + +func (s) TestBalancerExitIdle_All(t *testing.T) { + const balancerOne = "stub-balancer-test-balancer-group-exit-idle-one" + const balancerTwo = "stub-balancer-test-balancer-group-exit-idle-two" + const balancerThree = "stub-balancer-test-balancer-group-exit-idle-three" + + balancerNames := []string{balancerOne, balancerTwo, balancerThree} + testIDs := []string{testBalancerIDs[0], testBalancerIDs[1], testBalancerIDs[2]} + + exitIdleCh := make(chan string, len(balancerNames)) + + for _, name := range balancerNames { + stub.Register(name, stub.BalancerFuncs{ + ExitIdle: func(_ *stub.BalancerData) { + exitIdleCh <- name + }, + }) + } + + cc := testutils.NewBalancerClientConn(t) + bg := New(Options{ + CC: cc, + BuildOpts: balancer.BuildOptions{}, + StateAggregator: nil, + Logger: nil, + }) + defer bg.Close() + + bg.Add(testIDs[0], balancer.Get(balancerOne)) + bg.Add(testIDs[1], balancer.Get(balancerTwo)) + bg.Add(testIDs[2], balancer.Get(balancerThree)) + + bg.ExitIdle() + + called := make(map[string]bool) + for i := 0; i < len(balancerNames); i++ { + select { + case name := <-exitIdleCh: + called[name] = true + case <-time.After(time.Second): + t.Fatalf("Timeout: ExitIdle not called for all sub-balancers, got %d/%d", len(called), len(balancerNames)) + } + } + + for _, expected := range balancerNames { + if !called[expected] { + t.Errorf("ExitIdle was not called for sub-balancer registered as %q", expected) + } + } +} + +func (s) TestBalancerGroup_ExitIdle_AfterClose(t *testing.T) { + const balancerName = "stub-balancer-test-balancer-group-exit-idle-after-close" + + called := false + + stub.Register(balancerName, stub.BalancerFuncs{ + ExitIdle: func(_ *stub.BalancerData) { + called = true + }, + }) + + cc := testutils.NewBalancerClientConn(t) + bg := New(Options{ + CC: cc, + BuildOpts: balancer.BuildOptions{}, + StateAggregator: nil, + Logger: nil, + }) + + bg.Add(testBalancerIDs[0], balancer.Get(balancerName)) + + bg.Close() + + bg.ExitIdle() + + if called { + t.Fatalf("ExitIdle was called on sub-balancer even after BalancerGroup was closed") + } +} From df3f0f0ea19aa9d067c124abb2797997d221522c Mon Sep 17 00:00:00 2001 From: hoo Date: Sun, 1 Jun 2025 00:47:03 +0900 Subject: [PATCH 02/13] add tests for `ExitIdleOne` --- internal/balancergroup/balancergroup_test.go | 66 +++++++++++++++++--- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index 25529b66ba0b..cde650c1a95d 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -514,6 +514,58 @@ func (s) TestBalancerExitIdleOne(t *testing.T) { } } +func (s) TestBalancerGroup_ExitIdleOne_AfterClose(t *testing.T) { + balancerName := "stub-balancer-test-exit-idle-one-after-close" + called := false + + stub.Register(balancerName, stub.BalancerFuncs{ + ExitIdle: func(_ *stub.BalancerData) { + called = true + }, + }) + + bg := New(Options{ + CC: testutils.NewBalancerClientConn(t), + BuildOpts: balancer.BuildOptions{}, + StateAggregator: nil, + Logger: nil, + }) + + bg.Add(testBalancerIDs[0], balancer.Get(balancerName)) + bg.Close() + bg.ExitIdleOne(testBalancerIDs[0]) + + if called { + t.Fatalf("ExitIdleOne called ExitIdle on sub-balancer after BalancerGroup was closed") + } +} + +func (s) TestBalancerGroup_ExitIdleOne_NonExistentID(t *testing.T) { + balancerName := "stub-balancer-test-exit-idle-one-missing-id" + called := false + + stub.Register(balancerName, stub.BalancerFuncs{ + ExitIdle: func(_ *stub.BalancerData) { + called = true + }, + }) + + bg := New(Options{ + CC: testutils.NewBalancerClientConn(t), + BuildOpts: balancer.BuildOptions{}, + StateAggregator: nil, + Logger: nil, + }) + defer bg.Close() + + bg.Add(testBalancerIDs[0], balancer.Get(balancerName)) + bg.ExitIdleOne("non-existent-id") + + if called { + t.Fatalf("ExitIdleOne called ExitIdle on wrong sub-balancer ID") + } +} + // TestBalancerGracefulSwitch tests the graceful switch functionality for a // child of the balancer group. At first, the child is configured as a round // robin load balancer, and thus should behave accordingly. The test then @@ -642,9 +694,9 @@ func (s) TestBalancerGracefulSwitch(t *testing.T) { } func (s) TestBalancerExitIdle_All(t *testing.T) { - const balancerOne = "stub-balancer-test-balancer-group-exit-idle-one" - const balancerTwo = "stub-balancer-test-balancer-group-exit-idle-two" - const balancerThree = "stub-balancer-test-balancer-group-exit-idle-three" + balancerOne := "stub-balancer-test-balancer-group-exit-idle-one" + balancerTwo := "stub-balancer-test-balancer-group-exit-idle-two" + balancerThree := "stub-balancer-test-balancer-group-exit-idle-three" balancerNames := []string{balancerOne, balancerTwo, balancerThree} testIDs := []string{testBalancerIDs[0], testBalancerIDs[1], testBalancerIDs[2]} @@ -692,8 +744,7 @@ func (s) TestBalancerExitIdle_All(t *testing.T) { } func (s) TestBalancerGroup_ExitIdle_AfterClose(t *testing.T) { - const balancerName = "stub-balancer-test-balancer-group-exit-idle-after-close" - + balancerName := "stub-balancer-test-balancer-group-exit-idle-after-close" called := false stub.Register(balancerName, stub.BalancerFuncs{ @@ -702,18 +753,15 @@ func (s) TestBalancerGroup_ExitIdle_AfterClose(t *testing.T) { }, }) - cc := testutils.NewBalancerClientConn(t) bg := New(Options{ - CC: cc, + CC: testutils.NewBalancerClientConn(t), BuildOpts: balancer.BuildOptions{}, StateAggregator: nil, Logger: nil, }) bg.Add(testBalancerIDs[0], balancer.Get(balancerName)) - bg.Close() - bg.ExitIdle() if called { From 6ed4f3b2e5d0e6f05e66fcab7a648b7254ef82ed Mon Sep 17 00:00:00 2001 From: hoo Date: Sun, 1 Jun 2025 00:58:01 +0900 Subject: [PATCH 03/13] add tests for `ResolveError`, `UpdateClientConnState` --- internal/balancergroup/balancergroup_test.go | 57 ++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index cde650c1a95d..7dae4fdfbac6 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -484,6 +484,63 @@ func (s) TestBalancerGroupBuildOptions(t *testing.T) { } } +func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { + balancerName := "stub-balancer-test-update-client-state-after-close" + called := false + + stub.Register(balancerName, stub.BalancerFuncs{ + UpdateClientConnState: func(_ *stub.BalancerData, _ balancer.ClientConnState) error { + called = true + return nil + }, + }) + + bg := New(Options{ + CC: testutils.NewBalancerClientConn(t), + BuildOpts: balancer.BuildOptions{}, + StateAggregator: nil, + Logger: nil, + }) + + bg.Add(testBalancerIDs[0], balancer.Get(balancerName)) + bg.Close() + + err := bg.UpdateClientConnState(testBalancerIDs[0], balancer.ClientConnState{}) + if err != nil { + t.Fatalf("Expected nil error, got %v", err) + } + if called { + t.Fatalf("UpdateClientConnState was called after BalancerGroup was closed") + } +} + +func (s) TestBalancerGroup_ResolverError_AfterClose(t *testing.T) { + balancerName := "stub-balancer-test-resolver-error-after-close" + called := false + + stub.Register(balancerName, stub.BalancerFuncs{ + ResolverError: func(_ *stub.BalancerData, _ error) { + called = true + }, + }) + + bg := New(Options{ + CC: testutils.NewBalancerClientConn(t), + BuildOpts: balancer.BuildOptions{}, + StateAggregator: nil, + Logger: nil, + }) + + bg.Add(testBalancerIDs[0], balancer.Get(balancerName)) + bg.Close() + + bg.ResolverError(errors.New("test error")) + + if called { + t.Fatalf("ResolverError was called on sub-balancer after BalancerGroup was closed") + } +} + func (s) TestBalancerExitIdleOne(t *testing.T) { const balancerName = "stub-balancer-test-balancergroup-exit-idle-one" exitIdleCh := make(chan struct{}, 1) From c74418dcc767ec776c49c7a6c867f6a59a787a2e Mon Sep 17 00:00:00 2001 From: "elric.lim" Date: Sun, 8 Jun 2025 16:56:48 +0900 Subject: [PATCH 04/13] fix: use channel instead to get signal --- internal/balancergroup/balancergroup_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index 7dae4fdfbac6..256287e7ffe6 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -573,11 +573,11 @@ func (s) TestBalancerExitIdleOne(t *testing.T) { func (s) TestBalancerGroup_ExitIdleOne_AfterClose(t *testing.T) { balancerName := "stub-balancer-test-exit-idle-one-after-close" - called := false + exitIdleCh := make(chan struct{}) stub.Register(balancerName, stub.BalancerFuncs{ ExitIdle: func(_ *stub.BalancerData) { - called = true + close(exitIdleCh) }, }) @@ -592,7 +592,9 @@ func (s) TestBalancerGroup_ExitIdleOne_AfterClose(t *testing.T) { bg.Close() bg.ExitIdleOne(testBalancerIDs[0]) - if called { + select { + case <-time.After(time.Second): + case <-exitIdleCh: t.Fatalf("ExitIdleOne called ExitIdle on sub-balancer after BalancerGroup was closed") } } @@ -802,11 +804,11 @@ func (s) TestBalancerExitIdle_All(t *testing.T) { func (s) TestBalancerGroup_ExitIdle_AfterClose(t *testing.T) { balancerName := "stub-balancer-test-balancer-group-exit-idle-after-close" - called := false + exitIdleCh := make(chan struct{}) stub.Register(balancerName, stub.BalancerFuncs{ ExitIdle: func(_ *stub.BalancerData) { - called = true + close(exitIdleCh) }, }) @@ -821,7 +823,9 @@ func (s) TestBalancerGroup_ExitIdle_AfterClose(t *testing.T) { bg.Close() bg.ExitIdle() - if called { + select { + case <-exitIdleCh: t.Fatalf("ExitIdle was called on sub-balancer even after BalancerGroup was closed") + case <-time.After(defaultTestTimeout): } } From f2b8e021daceef7b31230b4fb152d5381e813ff9 Mon Sep 17 00:00:00 2001 From: "elric.lim" Date: Sun, 8 Jun 2025 17:24:29 +0900 Subject: [PATCH 05/13] fix: combine checks with select sentence --- internal/balancergroup/balancergroup_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index 256287e7ffe6..c80605bc92bd 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -789,17 +789,14 @@ func (s) TestBalancerExitIdle_All(t *testing.T) { for i := 0; i < len(balancerNames); i++ { select { case name := <-exitIdleCh: + if called[name] { + t.Fatalf("ExitIdle was called multiple times for sub-balancer %q", name) + } called[name] = true case <-time.After(time.Second): t.Fatalf("Timeout: ExitIdle not called for all sub-balancers, got %d/%d", len(called), len(balancerNames)) } } - - for _, expected := range balancerNames { - if !called[expected] { - t.Errorf("ExitIdle was not called for sub-balancer registered as %q", expected) - } - } } func (s) TestBalancerGroup_ExitIdle_AfterClose(t *testing.T) { From 4994ba104c210b55823d7488bd4345a628faa5b8 Mon Sep 17 00:00:00 2001 From: hoo Date: Sun, 15 Jun 2025 23:27:49 +0900 Subject: [PATCH 06/13] TestBalancerGroup_UpdateClientConnState_AfterClose --- internal/balancergroup/balancergroup_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index c80605bc92bd..e667ccf471bc 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -486,11 +486,11 @@ func (s) TestBalancerGroupBuildOptions(t *testing.T) { func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { balancerName := "stub-balancer-test-update-client-state-after-close" - called := false + exitIdleCh := make(chan struct{}) stub.Register(balancerName, stub.BalancerFuncs{ UpdateClientConnState: func(_ *stub.BalancerData, _ balancer.ClientConnState) error { - called = true + exitIdleCh <- struct{}{} return nil }, }) @@ -509,8 +509,10 @@ func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { if err != nil { t.Fatalf("Expected nil error, got %v", err) } - if called { + select { + case <-exitIdleCh: t.Fatalf("UpdateClientConnState was called after BalancerGroup was closed") + case <-time.After(defaultTestShortTimeout): } } From 73ddce790a69ffcf338ace4c2b4c467506f1bbee Mon Sep 17 00:00:00 2001 From: hoo Date: Sun, 15 Jun 2025 23:59:53 +0900 Subject: [PATCH 07/13] fix UpdateClientConnState --- internal/balancergroup/balancergroup_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index e667ccf471bc..ec9397f29f59 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -485,8 +485,9 @@ func (s) TestBalancerGroupBuildOptions(t *testing.T) { } func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { - balancerName := "stub-balancer-test-update-client-state-after-close" - exitIdleCh := make(chan struct{}) + t.Parallel() + balancerName := t.Name() + exitIdleCh := make(chan struct{}, 1) stub.Register(balancerName, stub.BalancerFuncs{ UpdateClientConnState: func(_ *stub.BalancerData, _ balancer.ClientConnState) error { @@ -505,10 +506,10 @@ func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { bg.Add(testBalancerIDs[0], balancer.Get(balancerName)) bg.Close() - err := bg.UpdateClientConnState(testBalancerIDs[0], balancer.ClientConnState{}) - if err != nil { + if err := bg.UpdateClientConnState(testBalancerIDs[0], balancer.ClientConnState{}); err != nil { t.Fatalf("Expected nil error, got %v", err) } + select { case <-exitIdleCh: t.Fatalf("UpdateClientConnState was called after BalancerGroup was closed") From fd47c7d47dade37178c7bad30ad8b307d49cd69e Mon Sep 17 00:00:00 2001 From: hoo Date: Mon, 16 Jun 2025 00:11:59 +0900 Subject: [PATCH 08/13] fix UpdateClientConnState --- internal/balancergroup/balancergroup_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index ec9397f29f59..50ca806223f0 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc/balancer/weightedtarget/weightedaggregator" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/balancer/stub" "google.golang.org/grpc/internal/channelz" "google.golang.org/grpc/internal/grpctest" @@ -485,7 +486,6 @@ func (s) TestBalancerGroupBuildOptions(t *testing.T) { } func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { - t.Parallel() balancerName := t.Name() exitIdleCh := make(chan struct{}, 1) @@ -495,6 +495,7 @@ func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { return nil }, }) + internal.BalancerUnregister(balancerName) bg := New(Options{ CC: testutils.NewBalancerClientConn(t), From 347b549e2f85b9b1b83896b9eb4a2f5a1df2d0bb Mon Sep 17 00:00:00 2001 From: "elric.lim" Date: Sun, 20 Jul 2025 17:22:48 +0900 Subject: [PATCH 09/13] fix: test --- xds/server_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xds/server_test.go b/xds/server_test.go index 13d01a7b974e..c2cf080d29c9 100644 --- a/xds/server_test.go +++ b/xds/server_test.go @@ -382,7 +382,7 @@ func (s) TestServeSuccess(t *testing.T) { modeChangeCh := testutils.NewChannel() modeChangeOption := ServingModeCallback(func(addr net.Addr, args ServingModeChangeArgs) { t.Logf("Server mode change callback invoked for listener %q with mode %q and error %v", addr.String(), args.Mode, args.Err) - modeChangeCh.Send(args.Mode) + modeChangeCh.SendOrFail(args.Mode) }) server, err := NewGRPCServer(modeChangeOption, BootstrapContentsForTesting(bootstrapContents)) if err != nil { @@ -515,7 +515,7 @@ func (s) TestHandleListenerUpdate_NoXDSCreds(t *testing.T) { modeChangeCh := testutils.NewChannel() modeChangeOption := ServingModeCallback(func(addr net.Addr, args ServingModeChangeArgs) { t.Logf("Server mode change callback invoked for listener %q with mode %q and error %v", addr.String(), args.Mode, args.Err) - modeChangeCh.Send(args.Mode) + modeChangeCh.SendOrFail(args.Mode) }) server, err := NewGRPCServer(modeChangeOption, BootstrapContentsForTesting(bootstrapContents)) if err != nil { @@ -607,7 +607,7 @@ func (s) TestHandleListenerUpdate_ErrorUpdate(t *testing.T) { modeChangeCh := testutils.NewChannel() modeChangeOption := ServingModeCallback(func(addr net.Addr, args ServingModeChangeArgs) { t.Logf("Server mode change callback invoked for listener %q with mode %q and error %v", addr.String(), args.Mode, args.Err) - modeChangeCh.Send(args.Mode) + modeChangeCh.SendOrFail(args.Mode) }) server, err := NewGRPCServer(modeChangeOption, BootstrapContentsForTesting(bootstrapContents)) if err != nil { From 0fda6d9212dd51b844f93d34cf3a70e5618f6522 Mon Sep 17 00:00:00 2001 From: "elric.lim" Date: Sun, 20 Jul 2025 17:38:59 +0900 Subject: [PATCH 10/13] change to use channel instead boolean --- internal/balancergroup/balancergroup_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index a6914864e7f3..e14ff5583e76 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -520,11 +520,11 @@ func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { func (s) TestBalancerGroup_ResolverError_AfterClose(t *testing.T) { balancerName := "stub-balancer-test-resolver-error-after-close" - called := false + exitIdleCh := make(chan struct{}, 1) stub.Register(balancerName, stub.BalancerFuncs{ ResolverError: func(_ *stub.BalancerData, _ error) { - called = true + exitIdleCh <- struct{}{} }, }) @@ -540,8 +540,10 @@ func (s) TestBalancerGroup_ResolverError_AfterClose(t *testing.T) { bg.ResolverError(errors.New("test error")) - if called { + select { + case <-exitIdleCh: t.Fatalf("ResolverError was called on sub-balancer after BalancerGroup was closed") + case <-time.After(defaultTestShortTimeout): } } @@ -597,7 +599,7 @@ func (s) TestBalancerGroup_ExitIdleOne_AfterClose(t *testing.T) { bg.ExitIdleOne(testBalancerIDs[0]) select { - case <-time.After(time.Second): + case <-time.After(defaultTestTimeout): case <-exitIdleCh: t.Fatalf("ExitIdleOne called ExitIdle on sub-balancer after BalancerGroup was closed") } @@ -826,6 +828,6 @@ func (s) TestBalancerGroup_ExitIdle_AfterClose(t *testing.T) { select { case <-exitIdleCh: t.Fatalf("ExitIdle was called on sub-balancer even after BalancerGroup was closed") - case <-time.After(defaultTestTimeout): + case <-time.After(defaultTestShortTimeout): } } From cae4825abbeb0ea0613d293cb6a9a7dc800b6d5f Mon Sep 17 00:00:00 2001 From: "elric.lim" Date: Sun, 20 Jul 2025 17:50:13 +0900 Subject: [PATCH 11/13] rollback SendOrFail() --- xds/server_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xds/server_test.go b/xds/server_test.go index 9bb53793be70..ed0179b59e1a 100644 --- a/xds/server_test.go +++ b/xds/server_test.go @@ -33,6 +33,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/uuid" + "google.golang.org/grpc" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials/insecure" @@ -382,7 +383,7 @@ func (s) TestServeSuccess(t *testing.T) { modeChangeCh := testutils.NewChannel() modeChangeOption := ServingModeCallback(func(addr net.Addr, args ServingModeChangeArgs) { t.Logf("Server mode change callback invoked for listener %q with mode %q and error %v", addr.String(), args.Mode, args.Err) - modeChangeCh.SendOrFail(args.Mode) + modeChangeCh.Send(args.Mode) }) server, err := NewGRPCServer(modeChangeOption, BootstrapContentsForTesting(bootstrapContents)) if err != nil { @@ -515,7 +516,7 @@ func (s) TestHandleListenerUpdate_NoXDSCreds(t *testing.T) { modeChangeCh := testutils.NewChannel() modeChangeOption := ServingModeCallback(func(addr net.Addr, args ServingModeChangeArgs) { t.Logf("Server mode change callback invoked for listener %q with mode %q and error %v", addr.String(), args.Mode, args.Err) - modeChangeCh.SendOrFail(args.Mode) + modeChangeCh.Send(args.Mode) }) server, err := NewGRPCServer(modeChangeOption, BootstrapContentsForTesting(bootstrapContents)) if err != nil { From bd2f76183b0b74c3c8fdb9f491262c4ae38ac4f3 Mon Sep 17 00:00:00 2001 From: hoo Date: Wed, 30 Jul 2025 00:20:32 +0900 Subject: [PATCH 12/13] update test code --- internal/balancergroup/balancergroup_test.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index e14ff5583e76..f6a87dbe33c4 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -519,7 +519,7 @@ func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { } func (s) TestBalancerGroup_ResolverError_AfterClose(t *testing.T) { - balancerName := "stub-balancer-test-resolver-error-after-close" + balancerName := t.Name() exitIdleCh := make(chan struct{}, 1) stub.Register(balancerName, stub.BalancerFuncs{ @@ -578,7 +578,7 @@ func (s) TestBalancerExitIdleOne(t *testing.T) { } func (s) TestBalancerGroup_ExitIdleOne_AfterClose(t *testing.T) { - balancerName := "stub-balancer-test-exit-idle-one-after-close" + balancerName := t.Name() exitIdleCh := make(chan struct{}) stub.Register(balancerName, stub.BalancerFuncs{ @@ -599,7 +599,7 @@ func (s) TestBalancerGroup_ExitIdleOne_AfterClose(t *testing.T) { bg.ExitIdleOne(testBalancerIDs[0]) select { - case <-time.After(defaultTestTimeout): + case <-time.After(defaultTestShortTimeout): case <-exitIdleCh: t.Fatalf("ExitIdleOne called ExitIdle on sub-balancer after BalancerGroup was closed") } @@ -798,8 +798,14 @@ func (s) TestBalancerExitIdle_All(t *testing.T) { t.Fatalf("ExitIdle was called multiple times for sub-balancer %q", name) } called[name] = true - case <-time.After(time.Second): - t.Fatalf("Timeout: ExitIdle not called for all sub-balancers, got %d/%d", len(called), len(balancerNames)) + case <-time.After(defaultTestTimeout): + var balancers []string + for _, name := range balancerNames { + if !called[name] { + balancers = append(balancers, name) + } + } + t.Fatalf("Timeout waiting for ExitIdle. Missing calls from: %v", balancers) } } } From a32977304ca566441e7cadac8714d34729db28e2 Mon Sep 17 00:00:00 2001 From: hoo Date: Wed, 30 Jul 2025 00:38:13 +0900 Subject: [PATCH 13/13] remove balancer unregister --- internal/balancergroup/balancergroup_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index f6a87dbe33c4..c948703d0411 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -30,7 +30,6 @@ import ( "google.golang.org/grpc/balancer/weightedtarget/weightedaggregator" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/balancer/stub" "google.golang.org/grpc/internal/channelz" "google.golang.org/grpc/internal/grpctest" @@ -495,7 +494,6 @@ func (s) TestBalancerGroup_UpdateClientConnState_AfterClose(t *testing.T) { return nil }, }) - internal.BalancerUnregister(balancerName) bg := New(Options{ CC: testutils.NewBalancerClientConn(t),