task 81: replace test sleeps with deterministic sync

5 files changed, 88 insertions, 60 deletions

diff --git a/cmd/player/main.go b/cmd/player/main.go
index 66d2889..ea3de1b 100644
--- a/cmd/player/main.go
+++ b/cmd/player/main.go
@@ -35,20 +35,21 @@ func run(args []string) error {
 
 // appDeps bundles all wired service-layer dependencies.
 type appDeps struct {
-	store       repository.Store
-	hasher      auth.Hasher
-	sm          *auth.SessionManager
-	cfg         *internal.Config
-	clk         clock.Clock
-	mediaSvc    service.MediaService
-	adminSvc    service.AdminService
-	progressSvc service.ProgressService
-	authSvc     service.AuthService
-	podcastSvc  service.PodcastEpisodeService
-	scanner     scanner.Scanner
-	gcWorker    *service.GCWorker
-	logger      *slog.Logger
-	appCtx      context.Context
+	store          repository.Store
+	hasher         auth.Hasher
+	sm             *auth.SessionManager
+	cfg            *internal.Config
+	clk            clock.Clock
+	mediaSvc       service.MediaService
+	adminSvc       service.AdminService
+	progressSvc    service.ProgressService
+	authSvc        service.AuthService
+	podcastSvc     service.PodcastEpisodeService
+	scanner        scanner.Scanner
+	gcWorker       *service.GCWorker
+	logger         *slog.Logger
+	appCtx         context.Context
+	workersStarted chan<- struct{}
 }
 
 // parseVersionFlag parses CLI flags and returns whether --version was requested.
@@ -137,6 +138,12 @@ func startBackgroundWorkers(deps *appDeps) {
 			}
 		}
 	}()
+	if deps.workersStarted != nil {
+		select {
+		case deps.workersStarted <- struct{}{}:
+		default:
+		}
+	}
 }
 
 // ensureSignalChannel returns the provided channel or creates a new one wired
diff --git a/cmd/player/main_test.go b/cmd/player/main_test.go
index 907e320..21b6ff0 100644
--- a/cmd/player/main_test.go
+++ b/cmd/player/main_test.go
@@ -106,9 +106,6 @@ func TestRunWithSignal_NormalShutdown(t *testing.T) {
 		errCh <- runWithSignal([]string{}, sigCh)
 	}()
 
-	// Give the server a moment to start listening.
-	time.Sleep(500 * time.Millisecond)
-
 	// Send a synthetic signal to trigger shutdown.
 	sigCh <- syscall.SIGINT
 
@@ -137,7 +134,6 @@ func TestRunWithSignal_LogLevels(t *testing.T) {
 			go func() {
 				errCh <- runWithSignal([]string{}, sigCh)
 			}()
-			time.Sleep(200 * time.Millisecond)
 			sigCh <- syscall.SIGTERM
 
 			select {
@@ -257,10 +253,15 @@ func TestStartBackgroundWorkers_StartsAndStops(t *testing.T) {
 	defer cancel()
 
 	deps := wireDeps(cfg, store, logger, ctx)
+	workersStarted := make(chan struct{}, 1)
+	deps.workersStarted = workersStarted
 	startBackgroundWorkers(deps)
 
-	// Give the goroutines a moment to start.
-	time.Sleep(50 * time.Millisecond)
+	select {
+	case <-workersStarted:
+	case <-time.After(time.Second):
+		t.Fatal("timeout waiting for background workers to start")
+	}
 
 	// Cancel the app context; workers should exit.
 	cancel()
diff --git a/internal/model/scan_test.go b/internal/model/scan_test.go
index cc0ba6b..acd1dd1 100644
--- a/internal/model/scan_test.go
+++ b/internal/model/scan_test.go
@@ -2,8 +2,8 @@ package model
 
 import (
 	"errors"
+	"sync"
 	"testing"
-	"time"
 )
 
 func TestScanProgress_Start(t *testing.T) {
@@ -116,8 +116,11 @@ func TestScanProgress_ConcurrentAccess(t *testing.T) {
 	p.Start(2)
 	p.SetFilesTotal(100)
 
+	start := make(chan struct{})
 	done := make(chan struct{})
+	var copies sync.WaitGroup
 	go func() {
+		<-start
 		for i := 0; i < 50; i++ {
 			p.IncrementFile()
 		}
@@ -125,9 +128,15 @@ func TestScanProgress_ConcurrentAccess(t *testing.T) {
 	}()
 
 	for i := 0; i < 50; i++ {
-		_ = p.Copy()
-		time.Sleep(time.Microsecond)
-	}
+		copies.Add(1)
+		go func() {
+			defer copies.Done()
+			<-start
+			_ = p.Copy()
+		}()
+	}
+	close(start)
+	copies.Wait()
 	<-done
 
 	cp := p.Copy()
diff --git a/internal/service/admin_test.go b/internal/service/admin_test.go
index 17f5373..03a177b 100644
--- a/internal/service/admin_test.go
+++ b/internal/service/admin_test.go
@@ -22,6 +22,15 @@ func (f *fakeScanner) Scan(ctx context.Context, root string, progress *model.Sca
 	return nil
 }
 
+func setScanDoneCh(t *testing.T, svc AdminService, doneCh chan<- struct{}) {
+	t.Helper()
+	adminSvc, ok := svc.(*adminService)
+	if !ok {
+		t.Fatalf("expected *adminService, got %T", svc)
+	}
+	adminSvc.scanService.doneCh = doneCh
+}
+
 type fakeHasher struct {
 	fixed string
 	err   error
@@ -300,35 +309,29 @@ func TestAdminService_TriggerRescan_CancelsPrevious(t *testing.T) {
 
 func TestAdminService_TriggerRescan_FreshProgressPerScan(t *testing.T) {
 	ctx := context.Background()
+	started := make(chan struct{})
 	done := make(chan struct{})
 	sc := &fakeScanner{
 		scanFunc: func(_ context.Context, _ string, progress *model.ScanProgress) error {
 			progress.Start(5)
+			close(started)
 			<-done
 			return nil
 		},
 	}
 	svc := NewAdminService(&repository.MockStore{}, newMockClock(), &fakeHasher{fixed: "hash"}, sc, "/media", ctx)
+	scanDone := make(chan struct{}, 1)
+	setScanDoneCh(t, svc, scanDone)
 
 	if err := svc.TriggerRescan(ctx); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	// Wait for goroutine to start.
-	for {
-		p := svc.ScanProgress(ctx)
-		if p.Running {
-			break
-		}
-		time.Sleep(10 * time.Millisecond)
-	}
+	<-started
 	close(done)
-	// Wait for goroutine to finish.
-	for {
-		p := svc.ScanProgress(ctx)
-		if !p.Running {
-			break
-		}
-		time.Sleep(10 * time.Millisecond)
+	select {
+	case <-scanDone:
+	case <-time.After(time.Second):
+		t.Fatal("timeout waiting for first scan to finish")
 	}
 
 	p1 := svc.ScanProgress(ctx)
@@ -337,10 +340,12 @@ func TestAdminService_TriggerRescan_FreshProgressPerScan(t *testing.T) {
 	}
 
 	// Start a new scan on the same service with different progress.
+	started2 := make(chan struct{})
 	done2 := make(chan struct{})
 	sc2 := &fakeScanner{
 		scanFunc: func(_ context.Context, _ string, progress *model.ScanProgress) error {
 			progress.Start(10)
+			close(started2)
 			<-done2
 			return nil
 		},
@@ -348,25 +353,17 @@ func TestAdminService_TriggerRescan_FreshProgressPerScan(t *testing.T) {
 	// We replace the scanner field via reflection? No, easier: just create new service.
 	// Actually, the test verifies per-service fresh progress, so new service is fine.
 	svc2 := NewAdminService(&repository.MockStore{}, newMockClock(), &fakeHasher{fixed: "hash"}, sc2, "/media", ctx)
+	scanDone2 := make(chan struct{}, 1)
+	setScanDoneCh(t, svc2, scanDone2)
 	if err := svc2.TriggerRescan(ctx); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	// Wait for goroutine to start.
-	for {
-		p := svc2.ScanProgress(ctx)
-		if p.Running {
-			break
-		}
-		time.Sleep(10 * time.Millisecond)
-	}
+	<-started2
 	close(done2)
-	// Wait for goroutine to finish.
-	for {
-		p := svc2.ScanProgress(ctx)
-		if !p.Running {
-			break
-		}
-		time.Sleep(10 * time.Millisecond)
+	select {
+	case <-scanDone2:
+	case <-time.After(time.Second):
+		t.Fatal("timeout waiting for second scan to finish")
 	}
 
 	p2 := svc2.ScanProgress(ctx)
@@ -392,12 +389,14 @@ func TestAdminService_TriggerRescan_ConcurrentCalls(t *testing.T) {
 	var wg sync.WaitGroup
 	callCount := 0
 	var mu sync.Mutex
+	started := make(chan struct{}, 5)
 	sc := &fakeScanner{
 		scanFunc: func(scanCtx context.Context, _ string, progress *model.ScanProgress) error {
 			mu.Lock()
 			callCount++
 			mu.Unlock()
 			progress.Start(1)
+			started <- struct{}{}
 			<-scanCtx.Done()
 			return scanCtx.Err()
 		},
@@ -413,13 +412,13 @@ func TestAdminService_TriggerRescan_ConcurrentCalls(t *testing.T) {
 	}
 	wg.Wait()
 
-	// Wait for the final surviving goroutine to start.
-	for {
-		progress := svc.ScanProgress(ctx)
-		if progress.Running {
-			break
+	timeout := time.After(time.Second)
+	for !svc.ScanProgress(ctx).Running {
+		select {
+		case <-started:
+		case <-timeout:
+			t.Fatal("timeout waiting for a running scan")
 		}
-		time.Sleep(10 * time.Millisecond)
 	}
 
 	mu.Lock()
diff --git a/internal/service/scan.go b/internal/service/scan.go
index d019616..52bc5c5 100644
--- a/internal/service/scan.go
+++ b/internal/service/scan.go
@@ -22,6 +22,7 @@ type scanService struct {
 	scanCancel context.CancelFunc
 	progress   *model.ScanProgress
 	appCtx     context.Context // application-level context used to propagate shutdown cancellation
+	doneCh     chan<- struct{}
 }
 
 // NewScanService creates a ScanService.
@@ -64,6 +65,7 @@ func (s *scanService) TriggerRescan(ctx context.Context) error {
 			progress.Done(nil)
 			s.logger.Info("rescan completed")
 		}
+		s.notifyDone()
 	}()
 	return nil
 }
@@ -77,3 +79,13 @@ func (s *scanService) ScanProgress(ctx context.Context) model.ScanProgress {
 	}
 	return progress.Copy()
 }
+
+func (s *scanService) notifyDone() {
+	if s.doneCh == nil {
+		return
+	}
+	select {
+	case s.doneCh <- struct{}{}:
+	default:
+	}
+}