From 10370edf59b676bcefb99ea2cfbe9c082d3a23d1 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Fri, 22 Nov 2024 09:57:41 -0800 Subject: [PATCH] internal/commands/server: prevent test panic on timeout (#5249) Previously, if one of these tests failed to start their servers before the timeout, there was a risk that the goroutine started to run the server would invoke t.Errorf after the test itself had concluded. Replace t.Errorf with fmt.Printf to avoid the panic, and include the test name for debugging. --- .../commands/server/controller_db_swap_test.go | 8 ++------ .../server/controller_ratelimit_reload_test.go | 16 ++++++++-------- .../cmd/commands/server/listener_reload_test.go | 2 +- .../worker_initial_upstreams_reload_test.go | 2 +- .../commands/server/worker_tags_reload_test.go | 2 +- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/internal/cmd/commands/server/controller_db_swap_test.go b/internal/cmd/commands/server/controller_db_swap_test.go index 73abff3f6..f6ee0f49e 100644 --- a/internal/cmd/commands/server/controller_db_swap_test.go +++ b/internal/cmd/commands/server/controller_db_swap_test.go @@ -115,7 +115,7 @@ func TestReloadControllerDatabase(t *testing.T) { exitCode := cmd.Run(args) if exitCode != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() @@ -241,7 +241,6 @@ func TestReloadControllerDatabase_InvalidNewDatabaseState(t *testing.T) { cfgHcl := fmt.Sprintf(dbSwapConfig, urlA, controllerKey, workerAuthKey, recoveryKey) require.NoError(t, os.WriteFile(td+"/config.hcl", []byte(cfgHcl), 0o644)) - errCh := make(chan error, 1) wg := &sync.WaitGroup{} wg.Add(1) go func() { @@ -251,15 +250,12 @@ func TestReloadControllerDatabase_InvalidNewDatabaseState(t *testing.T) { exitCode := cmd.Run(args) if exitCode != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - errCh <- fmt.Errorf("got a non-zero exit status: %s", output) - close(errCh) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() // Wait until things are up and running (or timeout). select { - case err := <-errCh: - t.Fatal(err) case <-cmd.startedCh: case <-time.After(15 * time.Second): t.Fatal("timeout") diff --git a/internal/cmd/commands/server/controller_ratelimit_reload_test.go b/internal/cmd/commands/server/controller_ratelimit_reload_test.go index 278d00f1a..2fce90741 100644 --- a/internal/cmd/commands/server/controller_ratelimit_reload_test.go +++ b/internal/cmd/commands/server/controller_ratelimit_reload_test.go @@ -184,7 +184,7 @@ listener "tcp" { ` ) -func TestRealodControllerRateLimits(t *testing.T) { +func TestReloadControllerRateLimits(t *testing.T) { td := t.TempDir() controllerKey := config.DevKeyGeneration() @@ -209,7 +209,7 @@ func TestRealodControllerRateLimits(t *testing.T) { exitCode := cmd.Run(args) if exitCode != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() @@ -282,7 +282,7 @@ func TestRealodControllerRateLimits(t *testing.T) { wg.Wait() } -func TestRealodControllerRateLimitsSameConfig(t *testing.T) { +func TestReloadControllerRateLimitsSameConfig(t *testing.T) { td := t.TempDir() // Create and migrate database A and B. @@ -308,7 +308,7 @@ func TestRealodControllerRateLimitsSameConfig(t *testing.T) { exitCode := cmd.Run(args) if exitCode != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() @@ -377,7 +377,7 @@ func TestRealodControllerRateLimitsSameConfig(t *testing.T) { wg.Wait() } -func TestRealodControllerRateLimitsDisable(t *testing.T) { +func TestReloadControllerRateLimitsDisable(t *testing.T) { td := t.TempDir() controllerKey := config.DevKeyGeneration() @@ -402,7 +402,7 @@ func TestRealodControllerRateLimitsDisable(t *testing.T) { exitCode := cmd.Run(args) if exitCode != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() @@ -475,7 +475,7 @@ func TestRealodControllerRateLimitsDisable(t *testing.T) { wg.Wait() } -func TestRealodControllerRateLimitsEnable(t *testing.T) { +func TestReloadControllerRateLimitsEnable(t *testing.T) { td := t.TempDir() controllerKey := config.DevKeyGeneration() @@ -501,7 +501,7 @@ func TestRealodControllerRateLimitsEnable(t *testing.T) { exitCode := cmd.Run(args) if exitCode != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() diff --git a/internal/cmd/commands/server/listener_reload_test.go b/internal/cmd/commands/server/listener_reload_test.go index 809e6ca7c..a5472f450 100644 --- a/internal/cmd/commands/server/listener_reload_test.go +++ b/internal/cmd/commands/server/listener_reload_test.go @@ -132,7 +132,7 @@ func TestServer_ReloadListener(t *testing.T) { defer wg.Done() if code := cmd.Run(args); code != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() diff --git a/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go b/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go index 24b7b36fb..a16935ce9 100644 --- a/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go +++ b/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go @@ -75,7 +75,7 @@ func TestServer_ReloadInitialUpstreams(t *testing.T) { defer wg.Done() if code := cmd.Run(nil); code != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }() diff --git a/internal/cmd/commands/server/worker_tags_reload_test.go b/internal/cmd/commands/server/worker_tags_reload_test.go index 4159b9f18..b6f8729c4 100644 --- a/internal/cmd/commands/server/worker_tags_reload_test.go +++ b/internal/cmd/commands/server/worker_tags_reload_test.go @@ -87,7 +87,7 @@ func TestServer_ReloadWorkerTags(t *testing.T) { defer wg.Done() if code := cmd.Run(nil); code != 0 { output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String() - t.Errorf("got a non-zero exit status: %s", output) + fmt.Printf("%s: got a non-zero exit status: %s", t.Name(), output) } }()