internal/agent/runner/cancel_test.go

@@ -3,35 +3,16 @@ package runner
 import (
 	"context"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
 	"gitea.dcglab.co.uk/steve/restic-manager/internal/api"
 )
 
-// safeSender is a thread-safe variant of fakeSender. The cancel test
+// (fakeSender is defined in runner_test.go; it's already lock-protected
-// has the runner goroutine sending envelopes while the test goroutine
+// because the runner's stdout + stderr pump goroutines call Send
-// is reading the slice, so we need a mutex.
+// concurrently. The original local 'safeSender' here was a workaround
-type safeSender struct {
+// from before fakeSender itself grew the mutex.)
-	mu   sync.Mutex
-	envs []api.Envelope
-}
-
-func (s *safeSender) Send(e api.Envelope) error {
-	s.mu.Lock()
-	s.envs = append(s.envs, e)
-	s.mu.Unlock()
-	return nil
-}
-
-func (s *safeSender) snapshot() []api.Envelope {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	out := make([]api.Envelope, len(s.envs))
-	copy(out, s.envs)
-	return out
-}
 
 // TestRunBackupCanceledMidRunReportsCanceled spawns a backup against
 // a fake restic that sleeps for 30 seconds, cancels the context after
@@ -48,7 +29,7 @@ func TestRunBackupCanceledMidRunReportsCanceled(t *testing.T) {
 	// SIGKILL fallback path firing — slower and noisier.
 	bin := setupScript(t, `exec sleep 30`)
 
-	tx := &safeSender{}
+	tx := &fakeSender{}
 	r := New(Config{ResticBin: bin}, tx, 0)
 
 	ctx, cancel := context.WithCancel(context.Background())

internal/agent/runner/runner_test.go

@@ -4,20 +4,42 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 
 	"gitea.dcglab.co.uk/steve/restic-manager/internal/api"
 	"gitea.dcglab.co.uk/steve/restic-manager/internal/restic"
 )
 
-// fakeSender collects sent envelopes for assertions.
+// fakeSender collects sent envelopes for assertions. Lock-protected
-type fakeSender struct{ envs []api.Envelope }
+// because the runner's pumpStdout / pumpStderr goroutines call Send
+// concurrently — without the mutex, -race in CI flags every test
+// that exercises a Run* method with both pumps active.
+type fakeSender struct {
+	mu   sync.Mutex
+	envs []api.Envelope
+}
 
 func (s *fakeSender) Send(e api.Envelope) error {
+	s.mu.Lock()
 	s.envs = append(s.envs, e)
+	s.mu.Unlock()
 	return nil
 }
 
+// snapshot returns a copy of the captured envelopes safe to read
+// without holding the lock. Tests use this when iterating envs while
+// other goroutines may still be writing — though in practice all
+// runner Run* methods join their pumps before returning, so callers
+// can also read .envs directly post-return.
+func (s *fakeSender) snapshot() []api.Envelope {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	out := make([]api.Envelope, len(s.envs))
+	copy(out, s.envs)
+	return out
+}
+
 // setupScript writes a shell script (without shebang) to a temp dir,
 // names it "restic", makes it executable, and returns the path.
 //