From 422b7c020b3d4ecc167c4f0af77e12808936e930 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 11:08:46 +0000 Subject: [PATCH] Test shutdown exit codes, Sentry startup failure and the processing wait (closes #86) These tests fail until the change that follows: runApp and WaitForProcessing do not exist yet, the server has no shutdowner, and a Sentry DSN that cannot be used exits the process from a goroutine instead of failing the server's start hook. runApp must return the exit code a shutdown request carries, 0 without one, and 1 when the app fails to start or stop. A listen error must ask fx to shut down with exit code 1. WaitForProcessing must wait for an image being processed and report it when its context ends first. Model: opus-5-5 --- cmd/pixad/main_internal_test.go | 80 ++++++++++++++++ ...max_concurrent_processing_internal_test.go | 66 +++++++++++++ internal/server/shutdown_internal_test.go | 96 +++++++++++++++++++ 3 files changed, 242 insertions(+) create mode 100644 cmd/pixad/main_internal_test.go create mode 100644 internal/server/shutdown_internal_test.go diff --git a/cmd/pixad/main_internal_test.go b/cmd/pixad/main_internal_test.go new file mode 100644 index 0000000..7023805 --- /dev/null +++ b/cmd/pixad/main_internal_test.go @@ -0,0 +1,80 @@ +package main + +import ( + "errors" + "testing" + + "go.uber.org/fx" +) + +// errTestHook is the error returned by the test hooks that fail. +var errTestHook = errors.New("test hook failed") + +// TestRunAppExitCode checks the exit code runApp returns: the one a +// shutdown request carries, 0 for a request without one (as for SIGINT or +// SIGTERM), and 1 when the app fails to start or to stop. +func TestRunAppExitCode(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + hook func(shutdowner fx.Shutdowner) fx.Hook + want int + }{ + { + name: "shutdown requested with exit code 1", + hook: func(shutdowner fx.Shutdowner) fx.Hook { + return fx.StartHook(func() error { + return shutdowner.Shutdown(fx.ExitCode(1)) + }) + }, + want: 1, + }, + { + name: "shutdown requested without an exit code", + hook: func(shutdowner fx.Shutdowner) fx.Hook { + return fx.StartHook(func() error { + return shutdowner.Shutdown() + }) + }, + want: 0, + }, + { + name: "start fails", + hook: func(fx.Shutdowner) fx.Hook { + return fx.StartHook(func() error { return errTestHook }) + }, + want: 1, + }, + { + name: "stop fails", + hook: func(shutdowner fx.Shutdowner) fx.Hook { + return fx.StartStopHook( + func() error { return shutdowner.Shutdown() }, + func() error { return errTestHook }, + ) + }, + want: 1, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + app := fx.New( + fx.NopLogger, + fx.Invoke(func(lc fx.Lifecycle, shutdowner fx.Shutdowner) { + lc.Append(tc.hook(shutdowner)) + }), + ) + + got := runApp(app) + t.Logf("runApp() = %d", got) + + if got != tc.want { + t.Errorf("runApp() = %d, want %d", got, tc.want) + } + }) + } +} diff --git a/internal/imageprocessor/max_concurrent_processing_internal_test.go b/internal/imageprocessor/max_concurrent_processing_internal_test.go index dc826c4..ba5388c 100644 --- a/internal/imageprocessor/max_concurrent_processing_internal_test.go +++ b/internal/imageprocessor/max_concurrent_processing_internal_test.go @@ -300,3 +300,69 @@ func TestProcessReleasesSlotOnError(t *testing.T) { }) } } + +// TestWaitForProcessing holds a processing slot with a Process call that +// cannot finish reading its input. WaitForProcessing must report that image +// when its context ends first, wait for it otherwise, return 0 once it has +// finished, and give back the slots it took while waiting. +func TestWaitForProcessing(t *testing.T) { + t.Parallel() + + proc := New(Params{MaxConcurrentProcessing: 2}) + + gate := make(chan struct{}) + entered := make(chan struct{}, 1) + results := make(chan error, 1) + + openGate := sync.OnceFunc(func() { close(gate) }) + t.Cleanup(openGate) + + processInBackground(proc, &gatedReader{ + data: bytes.NewReader(createTestJPEG(t, 10, 10)), gate: gate, + entered: entered, counter: &readingCounter{}, + }, results) + waitForEntries(t, entered, 1) + + ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) + defer cancel() + + stillProcessing := proc.WaitForProcessing(ctx) + t.Logf("WaitForProcessing() after its context ended: %d", stillProcessing) + + if stillProcessing != 1 { + t.Errorf("WaitForProcessing() after its context ended = %d, want 1", + stillProcessing) + } + + waited := make(chan int, 1) + + go func() { waited <- proc.WaitForProcessing(t.Context()) }() + + select { + case got := <-waited: + t.Fatalf("WaitForProcessing() = %d while an image was being processed", + got) + case <-time.After(100 * time.Millisecond): + } + + openGate() + + err := <-results + if err != nil { + t.Errorf("Process() error = %v, want nil", err) + } + + select { + case got := <-waited: + if got != 0 { + t.Errorf("WaitForProcessing() once processing finished = %d, want 0", + got) + } + case <-time.After(5 * time.Second): + t.Fatal("WaitForProcessing() did not return once processing finished") + } + + if held := len(proc.processingSemaphore); held != 0 { + t.Errorf("%d slots still held after WaitForProcessing() returned", held) + } +} diff --git a/internal/server/shutdown_internal_test.go b/internal/server/shutdown_internal_test.go new file mode 100644 index 0000000..4087bb1 --- /dev/null +++ b/internal/server/shutdown_internal_test.go @@ -0,0 +1,96 @@ +package server + +import ( + "log/slog" + "net" + "testing" + "time" + + "go.uber.org/fx" + "go.uber.org/fx/fxtest" + + "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/globals" + "sneak.berlin/go/pixa/internal/logger" +) + +// shutdownRecorder is an fx.Shutdowner that sends the options of each +// shutdown request on requests. +type shutdownRecorder struct { + requests chan []fx.ShutdownOption +} + +func (r shutdownRecorder) Shutdown(opts ...fx.ShutdownOption) error { + r.requests <- opts + + return nil +} + +// TestSentryInitFailureFailsStartup checks that a Sentry DSN that cannot be +// used makes the server's start hook fail, so fx stops what has already +// started, instead of the process exiting from a goroutine. +func TestSentryInitFailureFailsStartup(t *testing.T) { + t.Parallel() + + lc := fxtest.NewLifecycle(t) + + log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}}) + if err != nil { + t.Fatalf("logger.New() error = %v", err) + } + + _, err = New(lc, Params{ + Logger: log, + Globals: &globals.Globals{Appname: "pixad"}, + Config: &config.Config{SentryDSN: "not-a-dsn"}, + }) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + err = lc.Start(t.Context()) + t.Logf("Start() error = %v", err) + + if err == nil { + t.Fatal("Start() error = nil, want the Sentry initialization error") + } +} + +// TestListenErrorRequestsShutdownWithExitCode1 occupies the server's port +// and checks that the listen error asks fx to shut down with exit code 1. +func TestListenErrorRequestsShutdownWithExitCode1(t *testing.T) { + t.Parallel() + + busy, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", ":0") + if err != nil { + t.Fatalf("Listen() error = %v", err) + } + + t.Cleanup(func() { _ = busy.Close() }) + + addr, ok := busy.Addr().(*net.TCPAddr) + if !ok { + t.Fatalf("listener address %v is not a TCP address", busy.Addr()) + } + + requests := make(chan []fx.ShutdownOption, 1) + s := &Server{ + log: slog.New(slog.DiscardHandler), + config: &config.Config{Port: addr.Port}, + shutdowner: shutdownRecorder{requests: requests}, + } + s.httpServer = s.newHTTPServer() + + go s.serveUntilShutdown() + + select { + case opts := <-requests: + t.Logf("shutdown options = %v", opts) + + if len(opts) != 1 || opts[0] != fx.ExitCode(1) { + t.Errorf("shutdown options = %v, want [fx.ExitCode(1)]", opts) + } + case <-time.After(5 * time.Second): + t.Fatal("no shutdown was requested after the listen error") + } +}