diff --git a/internal/monitoring/scheduler_test.go b/internal/monitoring/scheduler_test.go index 58c3a6c..b4f5f8f 100644 --- a/internal/monitoring/scheduler_test.go +++ b/internal/monitoring/scheduler_test.go @@ -1,11 +1,396 @@ package monitoring import ( + "context" "math/rand" "testing" "time" ) +// mockStalenessSource is a test implementation of StalenessSource +type mockStalenessSource struct { + scores map[string]float64 +} + +func (m mockStalenessSource) StalenessScore(instanceType InstanceType, instanceName string) (float64, bool) { + key := string(instanceType) + ":" + instanceName + score, ok := m.scores[key] + return score, ok +} + +// mockIntervalSelector is a test implementation of IntervalSelector +type mockIntervalSelector struct { + interval time.Duration +} + +func (m mockIntervalSelector) SelectInterval(req IntervalRequest) time.Duration { + return m.interval +} + +// mockTaskEnqueuer is a test implementation of TaskEnqueuer +type mockTaskEnqueuer struct { + tasks []ScheduledTask +} + +func (m *mockTaskEnqueuer) Enqueue(ctx context.Context, task ScheduledTask) error { + m.tasks = append(m.tasks, task) + return nil +} + +// TestNewAdaptiveScheduler tests constructor with various input combinations +func TestNewAdaptiveScheduler(t *testing.T) { + defaultCfg := DefaultSchedulerConfig() + + tests := []struct { + name string + cfg SchedulerConfig + staleness StalenessSource + interval IntervalSelector + enqueuer TaskEnqueuer + wantBaseInterval time.Duration + wantMinInterval time.Duration + wantMaxInterval time.Duration + }{ + { + name: "all valid parameters preserved", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: mockStalenessSource{scores: map[string]float64{"pve:test": 0.5}}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 10 * time.Second, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "zero BaseInterval gets default", + cfg: SchedulerConfig{ + BaseInterval: 0, + MinInterval: 10 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: defaultCfg.BaseInterval, + wantMinInterval: 10 * time.Second, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "negative BaseInterval gets default", + cfg: SchedulerConfig{ + BaseInterval: -5 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: defaultCfg.BaseInterval, + wantMinInterval: 10 * time.Second, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "zero MinInterval gets default", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 0, + MaxInterval: 2 * time.Minute, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: defaultCfg.MinInterval, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "negative MinInterval gets default", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: -5 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: defaultCfg.MinInterval, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "zero MaxInterval gets default", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: 0, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 10 * time.Second, + wantMaxInterval: defaultCfg.MaxInterval, + }, + { + name: "negative MaxInterval gets default", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: -5 * time.Second, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 10 * time.Second, + wantMaxInterval: defaultCfg.MaxInterval, + }, + { + name: "MaxInterval less than MinInterval gets default", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 2 * time.Minute, + MaxInterval: 30 * time.Second, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 2 * time.Minute, + wantMaxInterval: defaultCfg.MaxInterval, + }, + { + name: "nil staleness gets noopStalenessSource", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: nil, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 10 * time.Second, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "nil interval gets newAdaptiveIntervalSelector", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: mockStalenessSource{}, + interval: nil, + enqueuer: &mockTaskEnqueuer{}, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 10 * time.Second, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "nil enqueuer gets noopTaskEnqueuer", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: mockStalenessSource{}, + interval: mockIntervalSelector{interval: 15 * time.Second}, + enqueuer: nil, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 10 * time.Second, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "all nil dependencies get defaults", + cfg: SchedulerConfig{ + BaseInterval: 20 * time.Second, + MinInterval: 10 * time.Second, + MaxInterval: 2 * time.Minute, + }, + staleness: nil, + interval: nil, + enqueuer: nil, + wantBaseInterval: 20 * time.Second, + wantMinInterval: 10 * time.Second, + wantMaxInterval: 2 * time.Minute, + }, + { + name: "all zero config and nil dependencies get all defaults", + cfg: SchedulerConfig{}, + staleness: nil, + interval: nil, + enqueuer: nil, + wantBaseInterval: defaultCfg.BaseInterval, + wantMinInterval: defaultCfg.MinInterval, + wantMaxInterval: defaultCfg.MaxInterval, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheduler := NewAdaptiveScheduler(tt.cfg, tt.staleness, tt.interval, tt.enqueuer) + + if scheduler == nil { + t.Fatal("NewAdaptiveScheduler returned nil") + } + + // Verify config values + if scheduler.cfg.BaseInterval != tt.wantBaseInterval { + t.Errorf("BaseInterval = %v, want %v", scheduler.cfg.BaseInterval, tt.wantBaseInterval) + } + if scheduler.cfg.MinInterval != tt.wantMinInterval { + t.Errorf("MinInterval = %v, want %v", scheduler.cfg.MinInterval, tt.wantMinInterval) + } + if scheduler.cfg.MaxInterval != tt.wantMaxInterval { + t.Errorf("MaxInterval = %v, want %v", scheduler.cfg.MaxInterval, tt.wantMaxInterval) + } + + // Verify staleness is not nil + if scheduler.staleness == nil { + t.Error("staleness is nil, expected non-nil") + } + + // Verify interval is not nil + if scheduler.interval == nil { + t.Error("interval is nil, expected non-nil") + } + + // Verify enqueuer is not nil + if scheduler.enqueuer == nil { + t.Error("enqueuer is nil, expected non-nil") + } + + // Verify lastPlan is initialized + if scheduler.lastPlan == nil { + t.Error("lastPlan is nil, expected initialized map") + } + }) + } +} + +// TestNewAdaptiveScheduler_StalenessType verifies nil staleness becomes noopStalenessSource +func TestNewAdaptiveScheduler_StalenessType(t *testing.T) { + cfg := SchedulerConfig{ + BaseInterval: 10 * time.Second, + MinInterval: 5 * time.Second, + MaxInterval: 60 * time.Second, + } + + scheduler := NewAdaptiveScheduler(cfg, nil, nil, nil) + + // noopStalenessSource always returns (0, false) + score, ok := scheduler.staleness.StalenessScore(InstanceTypePVE, "test") + if ok { + t.Error("noopStalenessSource should return ok=false") + } + if score != 0 { + t.Errorf("noopStalenessSource should return score=0, got %v", score) + } +} + +// TestNewAdaptiveScheduler_IntervalType verifies nil interval becomes adaptiveIntervalSelector +func TestNewAdaptiveScheduler_IntervalType(t *testing.T) { + cfg := SchedulerConfig{ + BaseInterval: 10 * time.Second, + MinInterval: 5 * time.Second, + MaxInterval: 60 * time.Second, + } + + scheduler := NewAdaptiveScheduler(cfg, nil, nil, nil) + + // adaptiveIntervalSelector should return a duration within bounds + req := IntervalRequest{ + Now: time.Now(), + BaseInterval: cfg.BaseInterval, + MinInterval: cfg.MinInterval, + MaxInterval: cfg.MaxInterval, + StalenessScore: 0.5, + InstanceKey: "test-interval-type", + } + + interval := scheduler.interval.SelectInterval(req) + if interval < cfg.MinInterval || interval > cfg.MaxInterval { + t.Errorf("SelectInterval returned %v, expected between %v and %v", + interval, cfg.MinInterval, cfg.MaxInterval) + } +} + +// TestNewAdaptiveScheduler_EnqueuerType verifies nil enqueuer becomes noopTaskEnqueuer +func TestNewAdaptiveScheduler_EnqueuerType(t *testing.T) { + cfg := SchedulerConfig{ + BaseInterval: 10 * time.Second, + MinInterval: 5 * time.Second, + MaxInterval: 60 * time.Second, + } + + scheduler := NewAdaptiveScheduler(cfg, nil, nil, nil) + + // noopTaskEnqueuer.Enqueue should return nil (no error) + task := ScheduledTask{ + InstanceName: "test", + InstanceType: InstanceTypePVE, + NextRun: time.Now(), + Interval: 10 * time.Second, + } + + err := scheduler.enqueuer.Enqueue(context.Background(), task) + if err != nil { + t.Errorf("noopTaskEnqueuer.Enqueue should return nil, got %v", err) + } +} + +// TestNewAdaptiveScheduler_PreservesCustomDependencies verifies custom implementations are preserved +func TestNewAdaptiveScheduler_PreservesCustomDependencies(t *testing.T) { + cfg := SchedulerConfig{ + BaseInterval: 10 * time.Second, + MinInterval: 5 * time.Second, + MaxInterval: 60 * time.Second, + } + + customStaleness := mockStalenessSource{scores: map[string]float64{"pve:test": 0.75}} + customInterval := mockIntervalSelector{interval: 25 * time.Second} + customEnqueuer := &mockTaskEnqueuer{} + + scheduler := NewAdaptiveScheduler(cfg, customStaleness, customInterval, customEnqueuer) + + // Verify custom staleness is used + score, ok := scheduler.staleness.StalenessScore(InstanceTypePVE, "test") + if !ok || score != 0.75 { + t.Errorf("expected custom staleness to return (0.75, true), got (%v, %v)", score, ok) + } + + // Verify custom interval selector is used + req := IntervalRequest{ + Now: time.Now(), + BaseInterval: cfg.BaseInterval, + MinInterval: cfg.MinInterval, + MaxInterval: cfg.MaxInterval, + StalenessScore: 0.5, + InstanceKey: "test", + } + interval := scheduler.interval.SelectInterval(req) + if interval != 25*time.Second { + t.Errorf("expected custom interval selector to return 25s, got %v", interval) + } + + // Verify custom enqueuer is used + task := ScheduledTask{ + InstanceName: "test", + InstanceType: InstanceTypePVE, + NextRun: time.Now(), + Interval: 10 * time.Second, + } + _ = scheduler.enqueuer.Enqueue(context.Background(), task) + if len(customEnqueuer.tasks) != 1 { + t.Errorf("expected custom enqueuer to have 1 task, got %d", len(customEnqueuer.tasks)) + } +} + // TestClampFloat tests the clampFloat helper function func TestClampFloat(t *testing.T) { tests := []struct { diff --git a/internal/monitoring/task_queue_test.go b/internal/monitoring/task_queue_test.go index 728d8b2..b8f33f5 100644 --- a/internal/monitoring/task_queue_test.go +++ b/internal/monitoring/task_queue_test.go @@ -788,6 +788,91 @@ func TestTaskQueue_Remove(t *testing.T) { }) } +func TestTaskHeap_Pop(t *testing.T) { + t.Run("pop from empty heap returns nil", func(t *testing.T) { + h := &taskHeap{} + + result := h.Pop() + + if result != nil { + t.Errorf("Pop() = %v, want nil", result) + } + }) + + t.Run("pop from single-element heap returns that element and empties heap", func(t *testing.T) { + entry := &scheduledTaskEntry{ + task: ScheduledTask{ + InstanceName: "pve-1", + InstanceType: InstanceTypePVE, + NextRun: time.Now(), + Priority: 1.0, + }, + index: 0, + } + h := &taskHeap{entry} + + result := h.Pop() + + if result != entry { + t.Errorf("Pop() returned wrong entry") + } + if entry.index != -1 { + t.Errorf("entry.index = %d, want -1", entry.index) + } + if len(*h) != 0 { + t.Errorf("heap length = %d, want 0", len(*h)) + } + }) + + t.Run("pop from multi-element heap returns last element with index set to -1", func(t *testing.T) { + entry1 := &scheduledTaskEntry{ + task: ScheduledTask{ + InstanceName: "pve-1", + InstanceType: InstanceTypePVE, + NextRun: time.Now(), + Priority: 1.0, + }, + index: 0, + } + entry2 := &scheduledTaskEntry{ + task: ScheduledTask{ + InstanceName: "pve-2", + InstanceType: InstanceTypePVE, + NextRun: time.Now().Add(10 * time.Second), + Priority: 1.0, + }, + index: 1, + } + entry3 := &scheduledTaskEntry{ + task: ScheduledTask{ + InstanceName: "pve-3", + InstanceType: InstanceTypePVE, + NextRun: time.Now().Add(20 * time.Second), + Priority: 1.0, + }, + index: 2, + } + h := &taskHeap{entry1, entry2, entry3} + + result := h.Pop() + + // Pop returns the last element in the slice (entry3) + if result != entry3 { + t.Errorf("Pop() returned wrong entry, got %v want entry3", result) + } + if entry3.index != -1 { + t.Errorf("entry3.index = %d, want -1", entry3.index) + } + if len(*h) != 2 { + t.Errorf("heap length = %d, want 2", len(*h)) + } + // entry1 and entry2 should still be in the heap + if (*h)[0] != entry1 || (*h)[1] != entry2 { + t.Errorf("remaining heap entries are wrong") + } + }) +} + // verifyHeapInvariant checks that the heap maintains its invariants: // 1. len(entries) matches heap size // 2. Each entry's index matches its actual position in heap diff --git a/internal/monitoring/temperature_test.go b/internal/monitoring/temperature_test.go index 1513c09..b70edf4 100644 --- a/internal/monitoring/temperature_test.go +++ b/internal/monitoring/temperature_test.go @@ -1942,6 +1942,167 @@ func TestParseNVMeTemps_AppendsToExisting(t *testing.T) { } } +// ============================================================================= +// Tests for isProxyEnabled +// ============================================================================= + +func TestIsProxyEnabled_NilProxyClient(t *testing.T) { + tc := &TemperatureCollector{ + proxyClient: nil, + useProxy: true, // even if this is true, nil client should return false + } + + if tc.isProxyEnabled() { + t.Error("expected isProxyEnabled() to return false when proxyClient is nil") + } +} + +func TestIsProxyEnabled_UseProxyAlreadyTrue(t *testing.T) { + stub := &stubTemperatureProxy{} + stub.setAvailable(false) // shouldn't matter since useProxy is already true + + tc := &TemperatureCollector{ + proxyClient: stub, + useProxy: true, + } + + if !tc.isProxyEnabled() { + t.Error("expected isProxyEnabled() to return true when useProxy is already true") + } + + // Verify useProxy remains true (unchanged) + if !tc.useProxy { + t.Error("expected useProxy to remain true") + } +} + +func TestIsProxyEnabled_UseProxyFalseStillInCooldown(t *testing.T) { + stub := &stubTemperatureProxy{} + stub.setAvailable(true) // shouldn't be checked since still in cooldown + + cooldownTime := time.Now().Add(5 * time.Minute) + tc := &TemperatureCollector{ + proxyClient: stub, + useProxy: false, + proxyCooldownUntil: cooldownTime, + proxyFailures: 2, + } + + if tc.isProxyEnabled() { + t.Error("expected isProxyEnabled() to return false while in cooldown") + } + + // Verify state is unchanged + if tc.useProxy { + t.Error("expected useProxy to remain false during cooldown") + } + if tc.proxyFailures != 2 { + t.Errorf("expected proxyFailures to remain 2, got %d", tc.proxyFailures) + } + if !tc.proxyCooldownUntil.Equal(cooldownTime) { + t.Errorf("expected proxyCooldownUntil to remain unchanged") + } +} + +func TestIsProxyEnabled_CooldownExpiredProxyAvailable(t *testing.T) { + stub := &stubTemperatureProxy{} + stub.setAvailable(true) + + tc := &TemperatureCollector{ + proxyClient: stub, + useProxy: false, + proxyCooldownUntil: time.Now().Add(-time.Minute), // expired + proxyFailures: 2, + } + + if !tc.isProxyEnabled() { + t.Error("expected isProxyEnabled() to return true when cooldown expired and proxy available") + } + + // Verify state was restored + if !tc.useProxy { + t.Error("expected useProxy to be set to true after restoration") + } + if tc.proxyFailures != 0 { + t.Errorf("expected proxyFailures to be reset to 0, got %d", tc.proxyFailures) + } + if !tc.proxyCooldownUntil.IsZero() { + t.Errorf("expected proxyCooldownUntil to be zero after restoration, got %s", tc.proxyCooldownUntil) + } +} + +func TestIsProxyEnabled_CooldownExpiredProxyUnavailable(t *testing.T) { + stub := &stubTemperatureProxy{} + stub.setAvailable(false) + + expiredCooldown := time.Now().Add(-time.Minute) + tc := &TemperatureCollector{ + proxyClient: stub, + useProxy: false, + proxyCooldownUntil: expiredCooldown, + proxyFailures: 2, + } + + before := time.Now() + if tc.isProxyEnabled() { + t.Error("expected isProxyEnabled() to return false when proxy unavailable") + } + + // Verify state + if tc.useProxy { + t.Error("expected useProxy to remain false when proxy unavailable") + } + // Cooldown should be extended by proxyRetryInterval (5 minutes) + if !tc.proxyCooldownUntil.After(before) { + t.Errorf("expected proxyCooldownUntil to be extended into the future, got %s", tc.proxyCooldownUntil) + } + expectedMinCooldown := before.Add(proxyRetryInterval - time.Second) + if tc.proxyCooldownUntil.Before(expectedMinCooldown) { + t.Errorf("expected proxyCooldownUntil to be at least %s, got %s", expectedMinCooldown, tc.proxyCooldownUntil) + } +} + +func TestIsProxyEnabled_ZeroCooldownProxyAvailable(t *testing.T) { + stub := &stubTemperatureProxy{} + stub.setAvailable(true) + + tc := &TemperatureCollector{ + proxyClient: stub, + useProxy: false, + proxyCooldownUntil: time.Time{}, // zero value - time.Now().After(zero) is true + proxyFailures: 0, + } + + if !tc.isProxyEnabled() { + t.Error("expected isProxyEnabled() to return true with zero cooldown and available proxy") + } + + if !tc.useProxy { + t.Error("expected useProxy to be set to true") + } +} + +func TestIsProxyEnabled_ZeroCooldownProxyUnavailable(t *testing.T) { + stub := &stubTemperatureProxy{} + stub.setAvailable(false) + + tc := &TemperatureCollector{ + proxyClient: stub, + useProxy: false, + proxyCooldownUntil: time.Time{}, // zero value + } + + before := time.Now() + if tc.isProxyEnabled() { + t.Error("expected isProxyEnabled() to return false when proxy unavailable") + } + + // Should set a new cooldown since proxy is unavailable + if !tc.proxyCooldownUntil.After(before) { + t.Errorf("expected proxyCooldownUntil to be set in the future, got %s", tc.proxyCooldownUntil) + } +} + // Helper functions for test setup func intPtr(i int) *int {