From 5c6421a011254319afdd7c3b1a4dd25ad7fab6c7 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Mon, 21 Sep 2026 16:16:23 +0700 Subject: [PATCH] test(server): cover the env parsers and the drain-wait loop cmd/noitu-server had no tests at all: every env* helper is pure, and config parsing is exactly where a production misconfiguration hides. waitForGamesToFinish narrows its *wsapi.Server parameter to the single method it calls, so the poll-then-check timing can be driven by a fake without a live server behind it. --- server/cmd/noitu-server/main.go | 8 +- server/cmd/noitu-server/main_test.go | 199 +++++++++++++++++++++++++++ 2 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 server/cmd/noitu-server/main_test.go diff --git a/server/cmd/noitu-server/main.go b/server/cmd/noitu-server/main.go index 908b004..dbbec7d 100644 --- a/server/cmd/noitu-server/main.go +++ b/server/cmd/noitu-server/main.go @@ -169,6 +169,12 @@ func newDebugServer(addr string) *http.Server { return &http.Server{Addr: addr, Handler: mux, ReadHeaderTimeout: 10 * time.Second} } +// gameCounter is the drain loop's only dependency on *wsapi.Server, narrowed +// so the polling logic can be tested without a live server behind it. +type gameCounter interface { + LiveGameCount() int64 +} + // waitForGamesToFinish blocks until every room's game has ended or timeout // passes, whichever is first. timeout <= 0 returns immediately, which is // today's behaviour: rooms are told the server is restarting and torn down @@ -177,7 +183,7 @@ func newDebugServer(addr string) *http.Server { // Only games count, not lobbies: a room nobody has started a game in has // nothing a restart costs, and waiting for it would make every deploy sit out // somebody's abandoned tab for the full timeout. -func waitForGamesToFinish(api *wsapi.Server, timeout time.Duration) { +func waitForGamesToFinish(api gameCounter, timeout time.Duration) { if timeout <= 0 { return } diff --git a/server/cmd/noitu-server/main_test.go b/server/cmd/noitu-server/main_test.go new file mode 100644 index 0000000..0409a4f --- /dev/null +++ b/server/cmd/noitu-server/main_test.go @@ -0,0 +1,199 @@ +package main + +import ( + "sync" + "testing" + "time" +) + +// TestEnvFallsBackWhenUnsetOrBlank covers env's whole contract: an unset +// variable and a blank one are the same "nothing configured" signal, since an +// operator's empty override should not silently defeat a default. +func TestEnvFallsBackWhenUnsetOrBlank(t *testing.T) { + const key = "NOITU_TEST_ENV_STRING" + + if got := env(key, "fallback"); got != "fallback" { + t.Errorf("unset: env = %q, want the fallback", got) + } + + t.Setenv(key, " ") + if got := env(key, "fallback"); got != "fallback" { + t.Errorf("blank: env = %q, want the fallback", got) + } + + t.Setenv(key, " configured ") + if got := env(key, "fallback"); got != "configured" { + t.Errorf("set: env = %q, want the trimmed value", got) + } +} + +// TestEnvIntRejectsInvalidAndNegative: a production misconfiguration here +// must fall back loudly rather than silently becoming zero or panicking. +func TestEnvIntRejectsInvalidAndNegative(t *testing.T) { + const key = "NOITU_TEST_ENV_INT" + + if got := envInt(key, 7); got != 7 { + t.Errorf("unset: envInt = %d, want the fallback", got) + } + + tests := []struct { + name, value string + want int + }{ + {"valid", "42", 42}, + {"zero is a real value", "0", 0}, + {"negative rejected", "-1", 7}, + {"not a number rejected", "many", 7}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(key, tc.value) + if got := envInt(key, 7); got != tc.want { + t.Errorf("envInt(%q) = %d, want %d", tc.value, got, tc.want) + } + }) + } +} + +// TestEnvDurationRejectsInvalidAndZero: unlike envNonNegDuration, a plain +// timing variable rejects zero along with garbage — every one of today's +// durations (turn limit, grace) has to be positive to mean anything. +func TestEnvDurationRejectsInvalidAndZero(t *testing.T) { + const key = "NOITU_TEST_ENV_DURATION" + + if got := envDuration(key, time.Second); got != time.Second { + t.Errorf("unset: envDuration = %v, want the fallback", got) + } + + tests := []struct { + name, value string + want time.Duration + }{ + {"valid", "30s", 30 * time.Second}, + {"zero rejected", "0s", time.Second}, + {"negative rejected", "-5s", time.Second}, + {"not a duration rejected", "soon", time.Second}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(key, tc.value) + if got := envDuration(key, time.Second); got != tc.want { + t.Errorf("envDuration(%q) = %v, want %v", tc.value, got, tc.want) + } + }) + } +} + +// TestEnvNonNegDurationAcceptsZero is the one difference from envDuration: +// NOITU_DRAIN_TIMEOUT=0 is a deliberate "do not wait", not a typo, and must +// not be rejected the way a zero turn limit would be. +func TestEnvNonNegDurationAcceptsZero(t *testing.T) { + const key = "NOITU_TEST_ENV_NONNEG_DURATION" + + tests := []struct { + name, value string + want time.Duration + }{ + {"valid", "10s", 10 * time.Second}, + {"zero accepted", "0s", 0}, + {"negative rejected", "-1s", 5 * time.Second}, + {"not a duration rejected", "soon", 5 * time.Second}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(key, tc.value) + if got := envNonNegDuration(key, 5*time.Second); got != tc.want { + t.Errorf("envNonNegDuration(%q) = %v, want %v", tc.value, got, tc.want) + } + }) + } +} + +// TestEnvListSplitsAndTrims covers the shape NOITU_ALLOWED_ORIGINS and +// NOITU_TRUSTED_PROXIES both rely on: comma-separated, whitespace around each +// entry ignored, and empty entries dropped rather than becoming a stray "". +func TestEnvListSplitsAndTrims(t *testing.T) { + const key = "NOITU_TEST_ENV_LIST" + + if got := envList(key); got != nil { + t.Errorf("unset: envList = %v, want nil", got) + } + + t.Setenv(key, " a , b ,, c") + got := envList(key) + want := []string{"a", "b", "c"} + if len(got) != len(want) { + t.Fatalf("envList = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("envList[%d] = %q, want %q", i, got[i], want[i]) + } + } +} + +// fakeGameCounter answers LiveGameCount from a value a test can change while +// the drain loop is polling it, so the loop can be driven without a real +// server or rooms behind it. +type fakeGameCounter struct { + mu sync.Mutex + live int64 +} + +func (f *fakeGameCounter) set(n int64) { + f.mu.Lock() + defer f.mu.Unlock() + f.live = n +} + +func (f *fakeGameCounter) LiveGameCount() int64 { + f.mu.Lock() + defer f.mu.Unlock() + return f.live +} + +// TestWaitForGamesToFinishReturnsAsSoonAsTheCountReachesZero: the loop must +// not sit out its whole timeout once the last game has actually ended. +func TestWaitForGamesToFinishReturnsAsSoonAsTheCountReachesZero(t *testing.T) { + counter := &fakeGameCounter{live: 1} + + done := make(chan struct{}) + go func() { + waitForGamesToFinish(counter, time.Minute) + close(done) + }() + + time.Sleep(50 * time.Millisecond) + counter.set(0) + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("waitForGamesToFinish did not return once the count reached zero") + } +} + +// TestWaitForGamesToFinishRespectsTheTimeout: a game that never ends must not +// hang the drain forever — it has to give up once its budget is spent. +func TestWaitForGamesToFinishRespectsTheTimeout(t *testing.T) { + counter := &fakeGameCounter{live: 1} + + start := time.Now() + waitForGamesToFinish(counter, 250*time.Millisecond) + if elapsed := time.Since(start); elapsed < 250*time.Millisecond { + t.Errorf("returned after %v, want at least the 250ms timeout", elapsed) + } +} + +// TestWaitForGamesToFinishReturnsImmediatelyOnZeroTimeout: a zero or negative +// timeout is today's original behaviour — rooms are torn down with whatever +// they were doing rather than waited on at all. +func TestWaitForGamesToFinishReturnsImmediatelyOnZeroTimeout(t *testing.T) { + counter := &fakeGameCounter{live: 1} + + start := time.Now() + waitForGamesToFinish(counter, 0) + if elapsed := time.Since(start); elapsed > 100*time.Millisecond { + t.Errorf("a zero timeout took %v, want an immediate return", elapsed) + } +}