test: lock-protect fakeSender so -race CI passes
The CI runs go test with -race; the agent runner has two pump goroutines (pumpStdout + pumpStderr) writing through the sender concurrently, and the unprotected fakeSender slice append raced. The cancel_test had a local 'safeSender' workaround for the same issue; promote that mutex onto fakeSender itself so every test in the package is race-clean without per-test variants. - fakeSender grows mu sync.Mutex; Send takes/releases. New snapshot() helper for tests that want a stable copy. - cancel_test drops its local safeSender + sync import; uses fakeSender. Verified: go test -race ./... passes across all packages.
This commit is contained in:
@@ -3,35 +3,16 @@ package runner
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"gitea.dcglab.co.uk/steve/restic-manager/internal/api"
|
"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
|
// TestRunBackupCanceledMidRunReportsCanceled spawns a backup against
|
||||||
// a fake restic that sleeps for 30 seconds, cancels the context after
|
// 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.
|
// SIGKILL fallback path firing — slower and noisier.
|
||||||
bin := setupScript(t, `exec sleep 30`)
|
bin := setupScript(t, `exec sleep 30`)
|
||||||
|
|
||||||
tx := &safeSender{}
|
tx := &fakeSender{}
|
||||||
r := New(Config{ResticBin: bin}, tx, 0)
|
r := New(Config{ResticBin: bin}, tx, 0)
|
||||||
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
|||||||
@@ -4,20 +4,42 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"gitea.dcglab.co.uk/steve/restic-manager/internal/api"
|
"gitea.dcglab.co.uk/steve/restic-manager/internal/api"
|
||||||
"gitea.dcglab.co.uk/steve/restic-manager/internal/restic"
|
"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 {
|
func (s *fakeSender) Send(e api.Envelope) error {
|
||||||
|
s.mu.Lock()
|
||||||
s.envs = append(s.envs, e)
|
s.envs = append(s.envs, e)
|
||||||
|
s.mu.Unlock()
|
||||||
return nil
|
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,
|
// setupScript writes a shell script (without shebang) to a temp dir,
|
||||||
// names it "restic", makes it executable, and returns the path.
|
// names it "restic", makes it executable, and returns the path.
|
||||||
//
|
//
|
||||||
|
|||||||
Reference in New Issue
Block a user