From ea0c6c2d21dfb28e5b887f068e7fcf97c84c8d5b Mon Sep 17 00:00:00 2001 From: zirain Date: Wed, 4 Feb 2026 14:46:48 +0800 Subject: [PATCH 1/3] fix test race Signed-off-by: zirain --- internal/cmd/server_test.go | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/internal/cmd/server_test.go b/internal/cmd/server_test.go index 760f83cacc..91662f08ac 100644 --- a/internal/cmd/server_test.go +++ b/internal/cmd/server_test.go @@ -14,6 +14,7 @@ import ( "strings" "sync/atomic" "testing" + "time" "github.com/stretchr/testify/require" @@ -102,8 +103,11 @@ func TestCustomProviderCancelWhenStarting(t *testing.T) { _, configPath := testCustomProvider(t, false) errCh := make(chan error) ctx, cancel := context.WithCancel(t.Context()) + + // Use a buffer to avoid data races with t.Output() after test completion + var logBuffer bytes.Buffer go func() { - errCh <- server(ctx, t.Output(), t.Output(), configPath, testHook, nil) + errCh <- server(ctx, &logBuffer, &logBuffer, configPath, testHook, nil) }() go func() { cancel() @@ -111,6 +115,11 @@ func TestCustomProviderCancelWhenStarting(t *testing.T) { err := <-errCh require.NoError(t, err) + + // Cleanup: allow goroutines to finish before test completes + t.Cleanup(func() { + time.Sleep(100 * time.Millisecond) + }) } func TestCustomProviderFailedToStart(t *testing.T) { @@ -118,13 +127,21 @@ func TestCustomProviderFailedToStart(t *testing.T) { errCh := make(chan error) ctx, cancel := context.WithCancel(t.Context()) + + // Use a buffer to avoid data races with t.Output() after test completion + var logBuffer bytes.Buffer go func() { - errCh <- server(ctx, t.Output(), t.Output(), configPath, testHook, nil) + errCh <- server(ctx, &logBuffer, &logBuffer, configPath, testHook, nil) }() err := <-errCh cancel() require.Error(t, err, "failed to load TLS config") + + // Cleanup: allow goroutines to finish before test completes + t.Cleanup(func() { + time.Sleep(100 * time.Millisecond) + }) } func TestCustomProviderCancelWhenConfigReload(t *testing.T) { @@ -152,13 +169,20 @@ func TestCustomProviderCancelWhenConfigReload(t *testing.T) { }() } + // Use a buffer to avoid data races with t.Output() after test completion + var logBuffer bytes.Buffer go func() { - errCh <- server(ctx, t.Output(), t.Output(), configPath, hook, startedCallback) + errCh <- server(ctx, &logBuffer, &logBuffer, configPath, hook, startedCallback) }() err := <-errCh cancel() require.NoError(t, err) + + // Cleanup: allow goroutines to finish before test completes + t.Cleanup(func() { + time.Sleep(100 * time.Millisecond) + }) } func TestGetConfigValidate(t *testing.T) { From 913e0e9d3d20d098dff716d41a9664f353a68f04 Mon Sep 17 00:00:00 2001 From: zirain Date: Wed, 4 Feb 2026 15:19:37 +0800 Subject: [PATCH 2/3] use io.Discard Signed-off-by: zirain --- internal/cmd/server_test.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/internal/cmd/server_test.go b/internal/cmd/server_test.go index 91662f08ac..e3db487275 100644 --- a/internal/cmd/server_test.go +++ b/internal/cmd/server_test.go @@ -104,10 +104,9 @@ func TestCustomProviderCancelWhenStarting(t *testing.T) { errCh := make(chan error) ctx, cancel := context.WithCancel(t.Context()) - // Use a buffer to avoid data races with t.Output() after test completion - var logBuffer bytes.Buffer + // Use io.Discard to avoid data races (it's thread-safe unlike bytes.Buffer) go func() { - errCh <- server(ctx, &logBuffer, &logBuffer, configPath, testHook, nil) + errCh <- server(ctx, io.Discard, io.Discard, configPath, testHook, nil) }() go func() { cancel() @@ -128,10 +127,9 @@ func TestCustomProviderFailedToStart(t *testing.T) { errCh := make(chan error) ctx, cancel := context.WithCancel(t.Context()) - // Use a buffer to avoid data races with t.Output() after test completion - var logBuffer bytes.Buffer + // Use io.Discard to avoid data races (it's thread-safe unlike bytes.Buffer) go func() { - errCh <- server(ctx, &logBuffer, &logBuffer, configPath, testHook, nil) + errCh <- server(ctx, io.Discard, io.Discard, configPath, testHook, nil) }() err := <-errCh @@ -169,10 +167,9 @@ func TestCustomProviderCancelWhenConfigReload(t *testing.T) { }() } - // Use a buffer to avoid data races with t.Output() after test completion - var logBuffer bytes.Buffer + // Use io.Discard to avoid data races (it's thread-safe unlike bytes.Buffer) go func() { - errCh <- server(ctx, &logBuffer, &logBuffer, configPath, hook, startedCallback) + errCh <- server(ctx, io.Discard, io.Discard, configPath, hook, startedCallback) }() err := <-errCh From b282051f613f7e1e2f75f5a05dde7bfeb082eb15 Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 5 Feb 2026 16:54:41 +0800 Subject: [PATCH 3/3] use sync.WaitGroup Signed-off-by: zirain --- go.mod | 2 +- internal/cmd/server_test.go | 51 +++++++++++++------------------------ 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/go.mod b/go.mod index f4f15dedb3..48384f6bea 100644 --- a/go.mod +++ b/go.mod @@ -64,6 +64,7 @@ require ( go.uber.org/zap v1.27.1 golang.org/x/exp v0.0.0-20250718183923-645b1fa84792 golang.org/x/net v0.49.0 + golang.org/x/sync v0.19.0 gomodules.xyz/jsonpatch/v2 v2.5.0 gonum.org/v1/gonum v0.17.0 google.golang.org/genproto/googleapis/api v0.0.0-20260128011058-8636f8732409 @@ -293,7 +294,6 @@ require ( golang.org/x/crypto/x509roots/fallback v0.0.0-20250406160420-959f8f3db0fb // indirect golang.org/x/mod v0.32.0 // indirect golang.org/x/oauth2 v0.34.0 // indirect - golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/term v0.39.0 // indirect golang.org/x/text v0.33.0 // indirect diff --git a/internal/cmd/server_test.go b/internal/cmd/server_test.go index e3db487275..7010bbca98 100644 --- a/internal/cmd/server_test.go +++ b/internal/cmd/server_test.go @@ -14,9 +14,9 @@ import ( "strings" "sync/atomic" "testing" - "time" "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" "github.com/envoyproxy/gateway/internal/envoygateway/config" ) @@ -101,53 +101,44 @@ func testCustomProvider(t *testing.T, genCert bool) (string, string) { func TestCustomProviderCancelWhenStarting(t *testing.T) { _, configPath := testCustomProvider(t, false) - errCh := make(chan error) ctx, cancel := context.WithCancel(t.Context()) + var g errgroup.Group // Use io.Discard to avoid data races (it's thread-safe unlike bytes.Buffer) - go func() { - errCh <- server(ctx, io.Discard, io.Discard, configPath, testHook, nil) - }() + g.Go(func() error { + return server(ctx, io.Discard, io.Discard, configPath, testHook, nil) + }) go func() { cancel() }() - err := <-errCh + err := g.Wait() require.NoError(t, err) - - // Cleanup: allow goroutines to finish before test completes - t.Cleanup(func() { - time.Sleep(100 * time.Millisecond) - }) } func TestCustomProviderFailedToStart(t *testing.T) { _, configPath := testCustomProvider(t, false) - errCh := make(chan error) ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + var g errgroup.Group // Use io.Discard to avoid data races (it's thread-safe unlike bytes.Buffer) - go func() { - errCh <- server(ctx, io.Discard, io.Discard, configPath, testHook, nil) - }() + g.Go(func() error { + return server(ctx, io.Discard, io.Discard, configPath, testHook, nil) + }) - err := <-errCh - cancel() + err := g.Wait() require.Error(t, err, "failed to load TLS config") - - // Cleanup: allow goroutines to finish before test completes - t.Cleanup(func() { - time.Sleep(100 * time.Millisecond) - }) } func TestCustomProviderCancelWhenConfigReload(t *testing.T) { configHome, configPath := testCustomProvider(t, true) - errCh := make(chan error) ctx, cancel := context.WithCancel(t.Context()) + defer cancel() count := atomic.Int32{} + var g errgroup.Group hook := func(c context.Context, cfg *config.Server) error { if count.Add(1) >= 2 { t.Logf("Config reload triggered, cancelling context") @@ -168,18 +159,12 @@ func TestCustomProviderCancelWhenConfigReload(t *testing.T) { } // Use io.Discard to avoid data races (it's thread-safe unlike bytes.Buffer) - go func() { - errCh <- server(ctx, io.Discard, io.Discard, configPath, hook, startedCallback) - }() + g.Go(func() error { + return server(ctx, io.Discard, io.Discard, configPath, hook, startedCallback) + }) - err := <-errCh - cancel() + err := g.Wait() require.NoError(t, err) - - // Cleanup: allow goroutines to finish before test completes - t.Cleanup(func() { - time.Sleep(100 * time.Millisecond) - }) } func TestGetConfigValidate(t *testing.T) {