From 196389853e6f4c5ad74e5d536d5727b6cb258e7b Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 1 Dec 2025 19:50:42 +0000 Subject: [PATCH] test: Add tests for StalenessScore, parseCPUTemps, queueDockerStopCommand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - StalenessScore: 78.3%→95.7% (4 cases for metrics, future timestamp, defaults) - parseCPUTemps: 98.1%→100% (core temp exceeding package case) - queueDockerStopCommand: 76.9%→100% (12 cases for validation, status checks) --- internal/monitoring/docker_commands_test.go | 354 ++++++++++++++++++ internal/monitoring/staleness_tracker_test.go | 114 ++++++ internal/monitoring/temperature_test.go | 35 ++ 3 files changed, 503 insertions(+) diff --git a/internal/monitoring/docker_commands_test.go b/internal/monitoring/docker_commands_test.go index e0752e2..75d9704 100644 --- a/internal/monitoring/docker_commands_test.go +++ b/internal/monitoring/docker_commands_test.go @@ -365,6 +365,360 @@ func TestMarkFailed(t *testing.T) { }) } +func TestQueueDockerStopCommand(t *testing.T) { + t.Parallel() + + t.Run("empty hostID returns error", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + _, err := monitor.queueDockerStopCommand("") + if err == nil { + t.Fatal("expected error for empty hostID") + } + if !strings.Contains(err.Error(), "docker host id is required") { + t.Fatalf("expected 'docker host id is required' error, got %q", err.Error()) + } + }) + + t.Run("whitespace-only hostID returns error", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + _, err := monitor.queueDockerStopCommand(" ") + if err == nil { + t.Fatal("expected error for whitespace-only hostID") + } + if !strings.Contains(err.Error(), "docker host id is required") { + t.Fatalf("expected 'docker host id is required' error, got %q", err.Error()) + } + }) + + t.Run("host not found returns error", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + + _, err := monitor.queueDockerStopCommand("nonexistent-host") + if err == nil { + t.Fatal("expected error for nonexistent host") + } + if !strings.Contains(err.Error(), "not found") { + t.Fatalf("expected 'not found' error, got %q", err.Error()) + } + }) + + t.Run("existing command in Queued status returns error", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + host := models.DockerHost{ + ID: "host-queued", + Hostname: "node-queued", + DisplayName: "node-queued", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue first command + _, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("first queue should succeed: %v", err) + } + + // Try to queue second command + _, err = monitor.queueDockerStopCommand(host.ID) + if err == nil { + t.Fatal("expected error for existing queued command") + } + if !strings.Contains(err.Error(), "already has a command in progress") { + t.Fatalf("expected 'already has a command in progress' error, got %q", err.Error()) + } + }) + + t.Run("existing command in Dispatched status returns error", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + host := models.DockerHost{ + ID: "host-dispatched", + Hostname: "node-dispatched", + DisplayName: "node-dispatched", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue and dispatch command + _, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue should succeed: %v", err) + } + monitor.FetchDockerCommandForHost(host.ID) // This marks it as dispatched + + // Try to queue second command + _, err = monitor.queueDockerStopCommand(host.ID) + if err == nil { + t.Fatal("expected error for existing dispatched command") + } + if !strings.Contains(err.Error(), "already has a command in progress") { + t.Fatalf("expected 'already has a command in progress' error, got %q", err.Error()) + } + }) + + t.Run("existing command in Acknowledged status returns error", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + host := models.DockerHost{ + ID: "host-ack", + Hostname: "node-ack", + DisplayName: "node-ack", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue, dispatch, and acknowledge command + cmdStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue should succeed: %v", err) + } + monitor.FetchDockerCommandForHost(host.ID) + _, _, _, err = monitor.AcknowledgeDockerHostCommand(cmdStatus.ID, host.ID, DockerCommandStatusAcknowledged, "ack") + if err != nil { + t.Fatalf("acknowledge should succeed: %v", err) + } + + // Try to queue second command + _, err = monitor.queueDockerStopCommand(host.ID) + if err == nil { + t.Fatal("expected error for existing acknowledged command") + } + if !strings.Contains(err.Error(), "already has a command in progress") { + t.Fatalf("expected 'already has a command in progress' error, got %q", err.Error()) + } + }) + + t.Run("existing completed command allows new command", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + host := models.DockerHost{ + ID: "host-completed", + Hostname: "node-completed", + DisplayName: "node-completed", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue, dispatch, and complete command + cmdStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("first queue should succeed: %v", err) + } + monitor.FetchDockerCommandForHost(host.ID) + _, _, _, err = monitor.AcknowledgeDockerHostCommand(cmdStatus.ID, host.ID, DockerCommandStatusCompleted, "done") + if err != nil { + t.Fatalf("complete should succeed: %v", err) + } + + // Queue new command should succeed + newStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("second queue should succeed after completion: %v", err) + } + if newStatus.Status != DockerCommandStatusQueued { + t.Fatalf("expected queued status, got %s", newStatus.Status) + } + }) + + t.Run("existing failed command allows new command", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + host := models.DockerHost{ + ID: "host-failed", + Hostname: "node-failed", + DisplayName: "node-failed", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue, dispatch, and fail command + cmdStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("first queue should succeed: %v", err) + } + monitor.FetchDockerCommandForHost(host.ID) + _, _, _, err = monitor.AcknowledgeDockerHostCommand(cmdStatus.ID, host.ID, DockerCommandStatusFailed, "failed") + if err != nil { + t.Fatalf("fail should succeed: %v", err) + } + + // Queue new command should succeed + newStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("second queue should succeed after failure: %v", err) + } + if newStatus.Status != DockerCommandStatusQueued { + t.Fatalf("expected queued status, got %s", newStatus.Status) + } + }) + + t.Run("nil dockerCommands map gets initialized", func(t *testing.T) { + t.Parallel() + + state := models.NewState() + monitor := &Monitor{ + state: state, + removedDockerHosts: make(map[string]time.Time), + dockerCommands: nil, // Explicitly nil + dockerCommandIndex: make(map[string]string), + } + + host := models.DockerHost{ + ID: "host-nil-map", + Hostname: "node-nil-map", + DisplayName: "node-nil-map", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + _, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue should succeed: %v", err) + } + + if monitor.dockerCommands == nil { + t.Fatal("dockerCommands map should be initialized") + } + if _, exists := monitor.dockerCommands[host.ID]; !exists { + t.Fatal("command should be stored in dockerCommands map") + } + }) + + t.Run("nil dockerCommandIndex map gets initialized", func(t *testing.T) { + t.Parallel() + + state := models.NewState() + monitor := &Monitor{ + state: state, + removedDockerHosts: make(map[string]time.Time), + dockerCommands: make(map[string]*dockerHostCommand), + dockerCommandIndex: nil, // Explicitly nil + } + + host := models.DockerHost{ + ID: "host-nil-index", + Hostname: "node-nil-index", + DisplayName: "node-nil-index", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + cmdStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue should succeed: %v", err) + } + + if monitor.dockerCommandIndex == nil { + t.Fatal("dockerCommandIndex map should be initialized") + } + if _, exists := monitor.dockerCommandIndex[cmdStatus.ID]; !exists { + t.Fatal("command should be indexed in dockerCommandIndex map") + } + }) + + t.Run("successful queue returns correct status", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + host := models.DockerHost{ + ID: "host-success", + Hostname: "node-success", + DisplayName: "node-success", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + cmdStatus, err := monitor.queueDockerStopCommand(host.ID) + if err != nil { + t.Fatalf("queue should succeed: %v", err) + } + + // Verify returned status + if cmdStatus.ID == "" { + t.Fatal("command ID should not be empty") + } + if cmdStatus.Type != DockerCommandTypeStop { + t.Fatalf("expected type %q, got %q", DockerCommandTypeStop, cmdStatus.Type) + } + if cmdStatus.Status != DockerCommandStatusQueued { + t.Fatalf("expected status %q, got %q", DockerCommandStatusQueued, cmdStatus.Status) + } + if cmdStatus.Message != "Stopping agent" { + t.Fatalf("expected message 'Stopping agent', got %q", cmdStatus.Message) + } + if cmdStatus.CreatedAt.IsZero() { + t.Fatal("CreatedAt should be set") + } + if cmdStatus.UpdatedAt.IsZero() { + t.Fatal("UpdatedAt should be set") + } + if cmdStatus.ExpiresAt == nil { + t.Fatal("ExpiresAt should be set") + } + + // Verify state updates + hostState := findDockerHost(t, monitor, host.ID) + if !hostState.PendingUninstall { + t.Fatal("host should be marked as pending uninstall") + } + if hostState.Command == nil { + t.Fatal("host command should be set in state") + } + if hostState.Command.ID != cmdStatus.ID { + t.Fatalf("host command ID mismatch: expected %q, got %q", cmdStatus.ID, hostState.Command.ID) + } + + // Verify internal maps + if _, exists := monitor.dockerCommands[host.ID]; !exists { + t.Fatal("command should be in dockerCommands map") + } + if resolvedHost, exists := monitor.dockerCommandIndex[cmdStatus.ID]; !exists || resolvedHost != host.ID { + t.Fatalf("command index mismatch: expected %q, got %q (exists=%v)", host.ID, resolvedHost, exists) + } + }) + + t.Run("hostID with leading/trailing whitespace is normalized", func(t *testing.T) { + t.Parallel() + + monitor := newTestMonitorForCommands(t) + host := models.DockerHost{ + ID: "host-whitespace", + Hostname: "node-whitespace", + DisplayName: "node-whitespace", + Status: "online", + } + monitor.state.UpsertDockerHost(host) + + // Queue with whitespace-padded ID + cmdStatus, err := monitor.queueDockerStopCommand(" host-whitespace ") + if err != nil { + t.Fatalf("queue with whitespace should succeed: %v", err) + } + if cmdStatus.Status != DockerCommandStatusQueued { + t.Fatalf("expected queued status, got %s", cmdStatus.Status) + } + + // Verify command is stored under normalized ID + if _, exists := monitor.dockerCommands["host-whitespace"]; !exists { + t.Fatal("command should be stored under normalized host ID") + } + }) +} + func TestAcknowledgeDockerCommandErrorPaths(t *testing.T) { t.Parallel() diff --git a/internal/monitoring/staleness_tracker_test.go b/internal/monitoring/staleness_tracker_test.go index 0e5b756..082040d 100644 --- a/internal/monitoring/staleness_tracker_test.go +++ b/internal/monitoring/staleness_tracker_test.go @@ -358,6 +358,120 @@ func TestStalenessTracker_ConcurrentAccess(t *testing.T) { } } +func TestStalenessTracker_StalenessScore_ZeroMaxStaleUsesDefault(t *testing.T) { + // Create tracker and directly set maxStale to 0 to test the defensive fallback + tracker := &StalenessTracker{ + entries: make(map[string]FreshnessSnapshot), + maxStale: 0, // Force zero to test default fallback + } + + // Set a 2.5 minute old success + oldTime := time.Now().Add(-150 * time.Second) + tracker.setSnapshot(FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "zero-max-test", + LastSuccess: oldTime, + }) + + score, ok := tracker.StalenessScore(InstanceTypePVE, "zero-max-test") + if !ok { + t.Fatal("staleness score should be available") + } + + // With default 5 minute maxStale, 2.5 minutes should give ~0.5 score + expected := 150.0 / 300.0 // 150s / 300s (5 min) + tolerance := 0.05 + if score < expected-tolerance || score > expected+tolerance { + t.Errorf("staleness score = %f, want ~%f (using default 5 min maxStale)", score, expected) + } +} + +func TestStalenessTracker_StalenessScore_FutureLastSuccess(t *testing.T) { + tracker := NewStalenessTracker(nil) + + // Set LastSuccess in the future (clock skew scenario) + futureTime := time.Now().Add(1 * time.Hour) + tracker.setSnapshot(FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "future-instance", + LastSuccess: futureTime, + }) + + score, ok := tracker.StalenessScore(InstanceTypePVE, "future-instance") + if !ok { + t.Fatal("staleness score should be available") + } + + // Future timestamp should return 0 (not stale) + if score != 0 { + t.Errorf("staleness score = %f, want 0 for future LastSuccess", score) + } +} + +func TestStalenessTracker_StalenessScore_WithMetrics(t *testing.T) { + // Create a minimal PollMetrics with lastSuccessByKey support + pm := &PollMetrics{ + lastSuccessByKey: make(map[metricKey]time.Time), + } + + tracker := NewStalenessTracker(pm) + tracker.SetBounds(10*time.Second, 60*time.Second) + + // Set an old success time in the tracker + oldTime := time.Now().Add(-45 * time.Second) + tracker.setSnapshot(FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "metrics-test", + LastSuccess: oldTime, + }) + + // Store a newer time in metrics + newerTime := time.Now().Add(-15 * time.Second) + pm.storeLastSuccess("pve", "metrics-test", newerTime) + + score, ok := tracker.StalenessScore(InstanceTypePVE, "metrics-test") + if !ok { + t.Fatal("staleness score should be available") + } + + // Should use the newer time from metrics: 15s / 60s = 0.25 + expected := 15.0 / 60.0 + tolerance := 0.05 + if score < expected-tolerance || score > expected+tolerance { + t.Errorf("staleness score = %f, want ~%f (using metrics time)", score, expected) + } +} + +func TestStalenessTracker_StalenessScore_MetricsNotUsedWhenLastSuccessZero(t *testing.T) { + // Create a minimal PollMetrics with lastSuccessByKey support + pm := &PollMetrics{ + lastSuccessByKey: make(map[metricKey]time.Time), + } + + tracker := NewStalenessTracker(pm) + + // Set a snapshot with zero LastSuccess (error-only entry) + tracker.setSnapshot(FreshnessSnapshot{ + InstanceType: InstanceTypePVE, + Instance: "error-only", + LastError: time.Now(), + // LastSuccess is zero + }) + + // Store a time in metrics (shouldn't be used since tracker LastSuccess is zero) + pm.storeLastSuccess("pve", "error-only", time.Now().Add(-10*time.Second)) + + score, ok := tracker.StalenessScore(InstanceTypePVE, "error-only") + if !ok { + t.Fatal("staleness score should be available") + } + + // Should return 1.0 because LastSuccess is zero (metrics lookup is conditional on non-zero LastSuccess) + if score != 1.0 { + t.Errorf("staleness score = %f, want 1.0 when tracker LastSuccess is zero", score) + } +} + func TestTrackerKey(t *testing.T) { tests := []struct { instanceType InstanceType diff --git a/internal/monitoring/temperature_test.go b/internal/monitoring/temperature_test.go index 1065808..9df31c5 100644 --- a/internal/monitoring/temperature_test.go +++ b/internal/monitoring/temperature_test.go @@ -909,6 +909,41 @@ func TestParseSensorsJSON_MultipleSuperioCPUFields(t *testing.T) { } } +// TestParseSensorsJSON_CoreTempExceedsPackage tests that CPUMax is updated when a core temp exceeds package temp +func TestParseSensorsJSON_CoreTempExceedsPackage(t *testing.T) { + collector := &TemperatureCollector{} + + // Core 1 has a higher temp than Package id 0 + jsonStr := `{ + "coretemp-isa-0000": { + "Package id 0": {"temp1_input": 45.0}, + "Core 0": {"temp2_input": 42.0}, + "Core 1": {"temp3_input": 52.0} + } + }` + + temp, err := collector.parseSensorsJSON(jsonStr) + if err != nil { + t.Fatalf("unexpected error parsing sensors output: %v", err) + } + if temp == nil { + t.Fatalf("expected temperature struct, got nil") + } + if !temp.Available { + t.Fatalf("expected temperature to be available") + } + if temp.CPUPackage != 45.0 { + t.Fatalf("expected cpu package temperature 45.0, got %.2f", temp.CPUPackage) + } + // CPUMax should be the highest core temp (52.0), not the package temp (45.0) + if temp.CPUMax != 52.0 { + t.Fatalf("expected cpu max temperature to be highest core temp (52.0), got %.2f", temp.CPUMax) + } + if len(temp.Cores) != 2 { + t.Fatalf("expected two core temperatures, got %d", len(temp.Cores)) + } +} + // ============================================================================= // Unit tests for utility functions // =============================================================================