From 9d9470ae273cbc06a9be6ee4bdf56e5d0a09d9f7 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Fri, 15 Nov 2024 14:06:57 -0800 Subject: [PATCH] internal/commands/server: prevent test panic on timeout 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 73abff3f66..f6ee0f49e1 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 278d00f1a6..2fce90741e 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 809e6ca7cd..a5472f4501 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 24b7b36fb1..a16935ce93 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 4159b9f181..b6f8729c4b 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) } }()