eda033ff9b
Addresses errcheck, gosec, revive, staticcheck, and unused findings
across the codebase (unchecked error returns, unsafe file inclusion
warnings on operator/test-controlled paths, missing package comments,
unused parameters, deprecated API usage). Also fixes two suppression
comments that were silently no-ops due to wrong syntax (#nosec needs
a leading '#', nolint reasons need '//' not '--').
With the backlog clear, drop continue-on-error from the CI lint job
per the plan left in b4b4806.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
248 lines
7.5 KiB
Go
248 lines
7.5 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/igodwin/notifier/internal/domain"
|
|
"github.com/igodwin/notifier/internal/logging"
|
|
"github.com/igodwin/notifier/internal/notifier"
|
|
"github.com/igodwin/notifier/internal/queue"
|
|
)
|
|
|
|
// alwaysFailNotifier is a fake domain.Notifier that always reports failure
|
|
// while recording the wall-clock time of each Send call, so tests can assert
|
|
// on the spacing between retry attempts.
|
|
type alwaysFailNotifier struct {
|
|
mu sync.Mutex
|
|
calls []time.Time
|
|
}
|
|
|
|
func (n *alwaysFailNotifier) Send(_ context.Context, notification *domain.Notification) (*domain.NotificationResult, error) {
|
|
n.mu.Lock()
|
|
n.calls = append(n.calls, time.Now())
|
|
n.mu.Unlock()
|
|
|
|
return &domain.NotificationResult{
|
|
NotificationID: notification.ID,
|
|
Success: false,
|
|
Error: "simulated failure",
|
|
SentAt: time.Now(),
|
|
}, nil
|
|
}
|
|
|
|
func (n *alwaysFailNotifier) Type() domain.NotificationType { return domain.TypeStdout }
|
|
|
|
func (n *alwaysFailNotifier) Validate(_ *domain.Notification) error { return nil }
|
|
|
|
func (n *alwaysFailNotifier) Close() error { return nil }
|
|
|
|
func (n *alwaysFailNotifier) callTimes() []time.Time {
|
|
n.mu.Lock()
|
|
defer n.mu.Unlock()
|
|
return append([]time.Time(nil), n.calls...)
|
|
}
|
|
|
|
// createFailingTestService builds a NotificationService wired to the given
|
|
// (always failing) notifier instead of the stdout notifier used by
|
|
// createTestService.
|
|
func createFailingTestService(t *testing.T, fail domain.Notifier) *NotificationService {
|
|
t.Helper()
|
|
|
|
factory := notifier.NewFactory()
|
|
if err := factory.RegisterNotifier(domain.TypeStdout, "", fail); err != nil {
|
|
t.Fatalf("Failed to register notifier: %v", err)
|
|
}
|
|
|
|
q, err := queue.NewLocalQueue(&domain.LocalQueueConfig{BufferSize: 100})
|
|
if err != nil {
|
|
t.Fatalf("Failed to create queue: %v", err)
|
|
}
|
|
|
|
logger, err := logging.NewFromConfig("error", "stdout")
|
|
if err != nil {
|
|
t.Fatalf("Failed to create logger: %v", err)
|
|
}
|
|
|
|
return NewNotificationService(factory, q, 2, nil, nil, logger)
|
|
}
|
|
|
|
// waitForStatus polls GetNotification until it observes the notification in
|
|
// the given status, or fails the test after timeout.
|
|
func waitForStatus(ctx context.Context, t *testing.T, svc *NotificationService, id string, status domain.NotificationStatus, timeout time.Duration) *domain.Notification {
|
|
t.Helper()
|
|
|
|
deadline := time.Now().Add(timeout)
|
|
for time.Now().Before(deadline) {
|
|
n, err := svc.GetNotification(ctx, id)
|
|
if err == nil && n.Status == status {
|
|
return n
|
|
}
|
|
time.Sleep(10 * time.Millisecond)
|
|
}
|
|
|
|
t.Fatalf("notification %s did not reach status %s within %v", id, status, timeout)
|
|
return nil
|
|
}
|
|
|
|
// TestRetryBackoffExponentialDelaysRequeue verifies that, with the default
|
|
// (exponential) backoff mode, the delay between retry attempts roughly
|
|
// doubles each time rather than hammering the failing notifier in a tight
|
|
// loop.
|
|
func TestRetryBackoffExponentialDelaysRequeue(t *testing.T) {
|
|
fail := &alwaysFailNotifier{}
|
|
svc := createFailingTestService(t, fail)
|
|
svc.retryBaseDelay = 40 * time.Millisecond // tiny base delay for a fast test
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
if err := svc.Start(ctx); err != nil {
|
|
t.Fatalf("Failed to start service: %v", err)
|
|
}
|
|
defer func() { _ = svc.Stop() }()
|
|
|
|
notification := &domain.Notification{
|
|
ID: "backoff-exponential-1",
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusPending,
|
|
Recipients: []string{"test@example.com"},
|
|
MaxRetries: 3,
|
|
}
|
|
|
|
if _, err := svc.Send(ctx, notification); err != nil {
|
|
t.Fatalf("Send failed: %v", err)
|
|
}
|
|
|
|
waitForStatus(ctx, t, svc, notification.ID, domain.StatusFailed, 5*time.Second)
|
|
|
|
calls := fail.callTimes()
|
|
if len(calls) != 3 {
|
|
t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
|
|
}
|
|
|
|
gap1 := calls[1].Sub(calls[0])
|
|
gap2 := calls[2].Sub(calls[1])
|
|
|
|
// Expected gaps are ~40ms then ~80ms; use generous tolerances to absorb
|
|
// scheduler jitter while still proving a real, growing delay was applied.
|
|
if gap1 < 25*time.Millisecond {
|
|
t.Errorf("expected delay before 2nd attempt >= ~40ms, got %v", gap1)
|
|
}
|
|
if gap2 < gap1 {
|
|
t.Errorf("expected delay before 3rd attempt (%v) to be larger than before 2nd (%v)", gap2, gap1)
|
|
}
|
|
|
|
t.Logf("attempt gaps: %v, %v", gap1, gap2)
|
|
}
|
|
|
|
// TestRetryBackoffNoneIsImmediate verifies that retryBackoff "none" requeues
|
|
// immediately, ignoring the configured base delay entirely.
|
|
func TestRetryBackoffNoneIsImmediate(t *testing.T) {
|
|
fail := &alwaysFailNotifier{}
|
|
svc := createFailingTestService(t, fail)
|
|
svc.WithRetryBackoff("none")
|
|
svc.retryBaseDelay = 5 * time.Second // large, to prove "none" ignores it
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
if err := svc.Start(ctx); err != nil {
|
|
t.Fatalf("Failed to start service: %v", err)
|
|
}
|
|
defer func() { _ = svc.Stop() }()
|
|
|
|
notification := &domain.Notification{
|
|
ID: "backoff-none-1",
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusPending,
|
|
Recipients: []string{"test@example.com"},
|
|
MaxRetries: 3,
|
|
}
|
|
|
|
start := time.Now()
|
|
if _, err := svc.Send(ctx, notification); err != nil {
|
|
t.Fatalf("Send failed: %v", err)
|
|
}
|
|
|
|
waitForStatus(ctx, t, svc, notification.ID, domain.StatusFailed, 2*time.Second)
|
|
elapsed := time.Since(start)
|
|
|
|
if elapsed > 1*time.Second {
|
|
t.Errorf("expected immediate retries with backoff=none, took %v", elapsed)
|
|
}
|
|
|
|
calls := fail.callTimes()
|
|
if len(calls) != 3 {
|
|
t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
|
|
}
|
|
}
|
|
|
|
// TestRetryBackoffShutdownAbandonsCleanly verifies that Stop() does not block
|
|
// on (or leak) a goroutine that is waiting out a backoff delay: it should
|
|
// abandon the pending retry and mark the notification Failed instead.
|
|
func TestRetryBackoffShutdownAbandonsCleanly(t *testing.T) {
|
|
fail := &alwaysFailNotifier{}
|
|
svc := createFailingTestService(t, fail)
|
|
svc.retryBaseDelay = 5 * time.Second // long enough that shutdown lands mid-wait
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
if err := svc.Start(ctx); err != nil {
|
|
t.Fatalf("Failed to start service: %v", err)
|
|
}
|
|
|
|
notification := &domain.Notification{
|
|
ID: "backoff-shutdown-1",
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusPending,
|
|
Recipients: []string{"test@example.com"},
|
|
MaxRetries: 3,
|
|
}
|
|
|
|
if _, err := svc.Send(ctx, notification); err != nil {
|
|
t.Fatalf("Send failed: %v", err)
|
|
}
|
|
|
|
// Give the worker time to make its first (failing) attempt and enter the
|
|
// backoff wait.
|
|
deadline := time.Now().Add(2 * time.Second)
|
|
for time.Now().Before(deadline) && len(fail.callTimes()) == 0 {
|
|
time.Sleep(10 * time.Millisecond)
|
|
}
|
|
if len(fail.callTimes()) == 0 {
|
|
t.Fatal("notifier was never called; cannot test shutdown mid-backoff")
|
|
}
|
|
|
|
stopped := make(chan error, 1)
|
|
go func() {
|
|
stopped <- svc.Stop()
|
|
}()
|
|
|
|
select {
|
|
case err := <-stopped:
|
|
if err != nil {
|
|
t.Errorf("Stop returned error: %v", err)
|
|
}
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("Stop did not return promptly - possible goroutine leak in retry backoff")
|
|
}
|
|
|
|
n, err := svc.GetNotification(context.Background(), notification.ID)
|
|
if err != nil {
|
|
t.Fatalf("GetNotification failed after shutdown: %v", err)
|
|
}
|
|
if n.Status != domain.StatusFailed {
|
|
t.Errorf("expected notification to be marked Failed after shutdown abandon, got %s", n.Status)
|
|
}
|
|
|
|
// Only the one attempt made before shutdown should have happened; the
|
|
// pending retry must not have fired.
|
|
if calls := len(fail.callTimes()); calls != 1 {
|
|
t.Errorf("expected exactly 1 send attempt before shutdown abandoned the retry, got %d", calls)
|
|
}
|
|
}
|