Move background goroutine start out of wireDeps into startBackgroundWorkers (task z0)

2 files changed, 121 insertions, 17 deletions

diff --git a/cmd/player/main.go b/cmd/player/main.go
index 0a7bacb..0e96e5f 100644
--- a/cmd/player/main.go
+++ b/cmd/player/main.go
@@ -48,6 +48,7 @@ type appDeps struct {
 	scanner     scanner.Scanner
 	gcWorker    *service.GCWorker
 	logger      *slog.Logger
+	appCtx      context.Context
 }
 
 // parseVersionFlag parses CLI flags and returns whether --version was requested.
@@ -98,23 +99,6 @@ func wireDeps(cfg *internal.Config, store repository.Store, logger *slog.Logger,
 	podcastSvc := service.NewPodcastService(store, clk, cfg.MediaRoot, helper, prober, thumbGen, cfg.PodcastCheckMinutes)
 
 	gcWorker := service.NewGCWorker(store, clk, cfg.MediaRoot, time.Duration(cfg.GCIntervalMinutes)*time.Minute, logger)
-	gcWorker.Start()
-
-	// Start podcast feed background checker.
-	go func() {
-		ticker := time.NewTicker(time.Duration(cfg.PodcastCheckMinutes) * time.Minute)
-		defer ticker.Stop()
-		for {
-			select {
-			case <-ticker.C:
-				if err := podcastSvc.CheckFeeds(context.Background()); err != nil {
-					logger.Error("podcast feed check failed", "err", err)
-				}
-			case <-appCtx.Done():
-				return
-			}
-		}
-	}()
 
 	return &appDeps{
 		store:       store,
@@ -130,9 +114,31 @@ func wireDeps(cfg *internal.Config, store repository.Store, logger *slog.Logger,
 		scanner:     fsScanner,
 		gcWorker:    gcWorker,
 		logger:      logger,
+		appCtx:      appCtx,
 	}
 }
 
+// startBackgroundWorkers launches background goroutines (GC, podcast feed checker).
+func startBackgroundWorkers(deps *appDeps) {
+	deps.gcWorker.Start()
+
+	// Start podcast feed background checker.
+	go func() {
+		ticker := time.NewTicker(time.Duration(deps.cfg.PodcastCheckMinutes) * time.Minute)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-ticker.C:
+				if err := deps.podcastSvc.CheckFeeds(context.Background()); err != nil {
+					deps.logger.Error("podcast feed check failed", "err", err)
+				}
+			case <-deps.appCtx.Done():
+				return
+			}
+		}
+	}()
+}
+
 // ensureSignalChannel returns the provided channel or creates a new one wired
 // to OS interrupt signals.
 func ensureSignalChannel(sigCh <-chan os.Signal) <-chan os.Signal {
@@ -214,6 +220,7 @@ func runWithSignal(args []string, sigCh <-chan os.Signal) error {
 
 	deps := wireDeps(cfg, store, logger, appCtx)
 	defer deps.gcWorker.Stop()
+	startBackgroundWorkers(deps)
 
 	staticFS := http.Dir("web")
 	remuxer := probe.NewFFRemuxer()
diff --git a/cmd/player/main_test.go b/cmd/player/main_test.go
index 29d36f0..907e320 100644
--- a/cmd/player/main_test.go
+++ b/cmd/player/main_test.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"io"
 	"log/slog"
 	"os"
@@ -185,3 +186,99 @@ func TestRunWithSignal_ServerErrorPath(t *testing.T) {
 		t.Fatal("expected error when server cannot bind privileged port")
 	}
 }
+
+func TestWireDeps_DoesNotStartBackgroundWorkers(t *testing.T) {
+	// wireDeps must only construct dependencies; it must not start any background goroutines.
+	tmpDir := t.TempDir()
+	dbPath := filepath.Join(tmpDir, "test.db")
+	mediaRoot := filepath.Join(tmpDir, "media")
+	if err := os.MkdirAll(mediaRoot, 0o755); err != nil {
+		t.Fatalf("mkdir media root: %v", err)
+	}
+
+	store, err := repository.Open(dbPath)
+	if err != nil {
+		t.Fatalf("open db: %v", err)
+	}
+	defer func() {
+		if err := store.Close(); err != nil {
+			t.Logf("close db: %v", err)
+		}
+	}()
+
+	cfg := &internal.Config{
+		SessionTimeoutHours: 1,
+		GCIntervalMinutes:   1,
+		PodcastCheckMinutes: 1,
+		MediaRoot:           mediaRoot,
+	}
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	deps := wireDeps(cfg, store, logger, ctx)
+	if deps.gcWorker == nil {
+		t.Fatal("expected gcWorker to be non-nil")
+	}
+
+	// We call Stop() immediately. If Start() had been called this is safe (idempotent).
+	// If Start() was NOT called, the internal stopCh is still open, so Stop() must handle it gracefully.
+	deps.gcWorker.Stop()
+}
+
+func TestStartBackgroundWorkers_StartsAndStops(t *testing.T) {
+	// startBackgroundWorkers should launch background goroutines that exit
+	// cleanly when the app context is cancelled.
+	tmpDir := t.TempDir()
+	dbPath := filepath.Join(tmpDir, "test.db")
+	mediaRoot := filepath.Join(tmpDir, "media")
+	if err := os.MkdirAll(mediaRoot, 0o755); err != nil {
+		t.Fatalf("mkdir media root: %v", err)
+	}
+
+	store, err := repository.Open(dbPath)
+	if err != nil {
+		t.Fatalf("open db: %v", err)
+	}
+	defer func() {
+		if err := store.Close(); err != nil {
+			t.Logf("close db: %v", err)
+		}
+	}()
+
+	cfg := &internal.Config{
+		SessionTimeoutHours: 1,
+		GCIntervalMinutes:   1,
+		PodcastCheckMinutes: 1,
+		MediaRoot:           mediaRoot,
+	}
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	deps := wireDeps(cfg, store, logger, ctx)
+	startBackgroundWorkers(deps)
+
+	// Give the goroutines a moment to start.
+	time.Sleep(50 * time.Millisecond)
+
+	// Cancel the app context; workers should exit.
+	cancel()
+
+	// Stop the GC worker explicitly (safe and idempotent).
+	deps.gcWorker.Stop()
+
+	// No explicit assertion for goroutine exit beyond the fact that we have not leaked;
+	// the final goroutine dump check in the test run will catch leaks.
+}
+
+func TestStartBackgroundWorkers_NilDepsPanics(t *testing.T) {
+	// Verify defensive behaviour: passing a nil pointer should panic quickly
+	// so the bug is surfaced at start-up rather than later as a nil dereference.
+	defer func() {
+		if r := recover(); r == nil {
+			t.Fatal("expected panic when startBackgroundWorkers receives nil")
+		}
+	}()
+	startBackgroundWorkers(nil)
+}