From 44b67457417c5f93cc9db641e1a30b90c2a077f1 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 1 Dec 2025 18:20:12 +0000 Subject: [PATCH] test: Add tests for recordAuthFailure, shouldSkipProxyHost, recoverFromPanic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - recordAuthFailure: 53%→100% (8 tests covering failure counting, node removal) - shouldSkipProxyHost: 44%→100% (9 tests covering cooldown logic, state cleanup) - recoverFromPanic: 50%→100% (7 tests covering various panic value types) Monitoring package 45.3%→45.9% --- internal/monitoring/monitor_test.go | 315 ++++++++++++++++++++++++ internal/monitoring/temperature_test.go | 169 +++++++++++++ 2 files changed, 484 insertions(+) diff --git a/internal/monitoring/monitor_test.go b/internal/monitoring/monitor_test.go index 431d4d2..7821009 100644 --- a/internal/monitoring/monitor_test.go +++ b/internal/monitoring/monitor_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" ) func TestParseDurationEnv(t *testing.T) { @@ -389,3 +390,317 @@ func TestBaseIntervalForInstanceType(t *testing.T) { } }) } + +func TestRecordAuthFailure(t *testing.T) { + t.Run("empty nodeType uses instanceName as nodeID", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + } + + m.recordAuthFailure("myinstance", "") + + if _, exists := m.authFailures["myinstance"]; !exists { + t.Error("expected authFailures['myinstance'] to exist") + } + if m.authFailures["myinstance"] != 1 { + t.Errorf("expected authFailures['myinstance'] = 1, got %d", m.authFailures["myinstance"]) + } + }) + + t.Run("non-empty nodeType creates type-instance nodeID", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + } + + m.recordAuthFailure("myinstance", "pve") + + expectedID := "pve-myinstance" + if _, exists := m.authFailures[expectedID]; !exists { + t.Errorf("expected authFailures['%s'] to exist", expectedID) + } + if m.authFailures[expectedID] != 1 { + t.Errorf("expected authFailures['%s'] = 1, got %d", expectedID, m.authFailures[expectedID]) + } + }) + + t.Run("increments authFailures counter", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + } + + m.recordAuthFailure("node1", "pve") + m.recordAuthFailure("node1", "pve") + m.recordAuthFailure("node1", "pve") + + if m.authFailures["pve-node1"] != 3 { + t.Errorf("expected 3 failures, got %d", m.authFailures["pve-node1"]) + } + }) + + t.Run("records lastAuthAttempt timestamp", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + } + + before := time.Now() + m.recordAuthFailure("node1", "pbs") + after := time.Now() + + timestamp, exists := m.lastAuthAttempt["pbs-node1"] + if !exists { + t.Fatal("expected lastAuthAttempt['pbs-node1'] to exist") + } + if timestamp.Before(before) || timestamp.After(after) { + t.Errorf("timestamp %v not between %v and %v", timestamp, before, after) + } + }) + + t.Run("triggers removal at 5 failures for nodeType pve", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + config: &config.Config{ + PVEInstances: []config.PVEInstance{ + {Name: "pve1", Host: "192.168.1.1"}, + }, + }, + state: newMinimalState(), + } + + // Record 4 failures - should not trigger removal + for i := 0; i < 4; i++ { + m.recordAuthFailure("pve1", "pve") + } + if m.authFailures["pve-pve1"] != 4 { + t.Errorf("expected 4 failures, got %d", m.authFailures["pve-pve1"]) + } + + // 5th failure should trigger removal and reset counters + m.recordAuthFailure("pve1", "pve") + + if _, exists := m.authFailures["pve-pve1"]; exists { + t.Error("expected authFailures['pve-pve1'] to be deleted after 5 failures") + } + if _, exists := m.lastAuthAttempt["pve-pve1"]; exists { + t.Error("expected lastAuthAttempt['pve-pve1'] to be deleted after 5 failures") + } + }) + + t.Run("triggers removal at 5 failures for nodeType pbs", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + config: &config.Config{}, + state: newMinimalState(), + } + + // Record 5 failures + for i := 0; i < 5; i++ { + m.recordAuthFailure("pbs1", "pbs") + } + + // Counters should be reset after removal + if _, exists := m.authFailures["pbs-pbs1"]; exists { + t.Error("expected authFailures['pbs-pbs1'] to be deleted after 5 failures") + } + if _, exists := m.lastAuthAttempt["pbs-pbs1"]; exists { + t.Error("expected lastAuthAttempt['pbs-pbs1'] to be deleted after 5 failures") + } + }) + + t.Run("triggers removal at 5 failures for nodeType pmg", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + config: &config.Config{}, + state: newMinimalState(), + } + + // Record 5 failures + for i := 0; i < 5; i++ { + m.recordAuthFailure("pmg1", "pmg") + } + + // Counters should be reset after removal + if _, exists := m.authFailures["pmg-pmg1"]; exists { + t.Error("expected authFailures['pmg-pmg1'] to be deleted after 5 failures") + } + if _, exists := m.lastAuthAttempt["pmg-pmg1"]; exists { + t.Error("expected lastAuthAttempt['pmg-pmg1'] to be deleted after 5 failures") + } + }) + + t.Run("resets counters after removal", func(t *testing.T) { + m := &Monitor{ + authFailures: make(map[string]int), + lastAuthAttempt: make(map[string]time.Time), + config: &config.Config{}, + state: newMinimalState(), + } + + // Trigger removal with 5 failures + for i := 0; i < 5; i++ { + m.recordAuthFailure("testnode", "pve") + } + + // Verify counters are reset + if len(m.authFailures) != 0 { + t.Errorf("expected authFailures to be empty, got %v", m.authFailures) + } + if len(m.lastAuthAttempt) != 0 { + t.Errorf("expected lastAuthAttempt to be empty, got %v", m.lastAuthAttempt) + } + + // New failures should start from 1 again + m.recordAuthFailure("testnode", "pve") + if m.authFailures["pve-testnode"] != 1 { + t.Errorf("expected counter to restart at 1, got %d", m.authFailures["pve-testnode"]) + } + }) +} + +// newMinimalState creates a minimal State for testing +func newMinimalState() *models.State { + return models.NewState() +} + +func TestRecoverFromPanic(t *testing.T) { + t.Run("no panic does nothing", func(t *testing.T) { + // When no panic occurs, recoverFromPanic should do nothing + // and the function should complete normally + completed := false + func() { + defer recoverFromPanic("test-goroutine") + completed = true + }() + if !completed { + t.Error("expected function to complete normally without panic") + } + }) + + t.Run("recovers from string panic", func(t *testing.T) { + didPanic := false + recovered := false + func() { + defer func() { + // This runs after recoverFromPanic + recovered = true + }() + defer recoverFromPanic("test-goroutine") + didPanic = true + panic("test panic message") + }() + if !didPanic { + t.Error("expected panic to occur") + } + if !recovered { + t.Error("expected to recover from panic") + } + }) + + t.Run("recovers from error panic", func(t *testing.T) { + didPanic := false + recovered := false + testErr := &testError{msg: "test error"} + func() { + defer func() { + recovered = true + }() + defer recoverFromPanic("error-goroutine") + didPanic = true + panic(testErr) + }() + if !didPanic { + t.Error("expected panic to occur") + } + if !recovered { + t.Error("expected to recover from error panic") + } + }) + + t.Run("recovers from int panic", func(t *testing.T) { + didPanic := false + recovered := false + func() { + defer func() { + recovered = true + }() + defer recoverFromPanic("int-goroutine") + didPanic = true + panic(42) + }() + if !didPanic { + t.Error("expected panic to occur") + } + if !recovered { + t.Error("expected to recover from int panic") + } + }) + + t.Run("recovers from struct panic", func(t *testing.T) { + type panicData struct { + code int + message string + } + didPanic := false + recovered := false + func() { + defer func() { + recovered = true + }() + defer recoverFromPanic("struct-goroutine") + didPanic = true + panic(panicData{code: 500, message: "internal error"}) + }() + if !didPanic { + t.Error("expected panic to occur") + } + if !recovered { + t.Error("expected to recover from struct panic") + } + }) + + t.Run("recovers from nil panic", func(t *testing.T) { + didPanic := false + recovered := false + func() { + defer func() { + recovered = true + }() + defer recoverFromPanic("nil-goroutine") + didPanic = true + panic(nil) + }() + if !didPanic { + t.Error("expected panic to occur") + } + if !recovered { + t.Error("expected to recover from nil panic") + } + }) + + t.Run("code after panic is not executed", func(t *testing.T) { + afterPanicExecuted := false + func() { + defer recoverFromPanic("test-goroutine") + panic("stop here") + afterPanicExecuted = true //nolint:govet // unreachable code is intentional for test + }() + if afterPanicExecuted { + t.Error("expected code after panic to not execute") + } + }) +} + +// testError implements error interface for panic testing +type testError struct { + msg string +} + +func (e *testError) Error() string { + return e.msg +} diff --git a/internal/monitoring/temperature_test.go b/internal/monitoring/temperature_test.go index 70270c5..33d31a5 100644 --- a/internal/monitoring/temperature_test.go +++ b/internal/monitoring/temperature_test.go @@ -1317,6 +1317,175 @@ func TestNormalizeSMARTEntries(t *testing.T) { } } +// ============================================================================= +// Tests for shouldSkipProxyHost +// ============================================================================= + +func TestShouldSkipProxyHost_EmptyHost(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: make(map[string]*proxyHostState), + } + + if tc.shouldSkipProxyHost("") { + t.Error("expected empty host to return false") + } +} + +func TestShouldSkipProxyHost_WhitespaceOnlyHost(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: make(map[string]*proxyHostState), + } + + if tc.shouldSkipProxyHost(" ") { + t.Error("expected whitespace-only host (trimmed to empty) to return false") + } + if tc.shouldSkipProxyHost("\t\n") { + t.Error("expected tab/newline host (trimmed to empty) to return false") + } +} + +func TestShouldSkipProxyHost_NotInMap(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: make(map[string]*proxyHostState), + } + + if tc.shouldSkipProxyHost("192.168.1.100") { + t.Error("expected host not in map to return false") + } +} + +func TestShouldSkipProxyHost_NilState(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: map[string]*proxyHostState{ + "192.168.1.100": nil, + }, + } + + if tc.shouldSkipProxyHost("192.168.1.100") { + t.Error("expected host with nil state to return false") + } +} + +func TestShouldSkipProxyHost_ZeroCooldownUntil(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: map[string]*proxyHostState{ + "192.168.1.100": { + failures: 2, + cooldownUntil: time.Time{}, // zero value + }, + }, + } + + if tc.shouldSkipProxyHost("192.168.1.100") { + t.Error("expected host with zero cooldownUntil to return false") + } + + // Verify the host was cleaned up from the map + tc.proxyMu.Lock() + _, exists := tc.proxyHostStates["192.168.1.100"] + tc.proxyMu.Unlock() + + if exists { + t.Error("expected host with zero cooldownUntil to be deleted from map") + } +} + +func TestShouldSkipProxyHost_ExpiredCooldown(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: map[string]*proxyHostState{ + "192.168.1.100": { + failures: 2, + cooldownUntil: time.Now().Add(-time.Minute), // expired + lastError: "some error", + }, + }, + } + + if tc.shouldSkipProxyHost("192.168.1.100") { + t.Error("expected host with expired cooldown to return false") + } + + // Verify the state was reset and host was deleted + tc.proxyMu.Lock() + _, exists := tc.proxyHostStates["192.168.1.100"] + tc.proxyMu.Unlock() + + if exists { + t.Error("expected host with expired cooldown to be deleted from map") + } +} + +func TestShouldSkipProxyHost_ActiveCooldown(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: map[string]*proxyHostState{ + "192.168.1.100": { + failures: 0, + cooldownUntil: time.Now().Add(5 * time.Minute), // active + lastError: "connection refused", + }, + }, + } + + if !tc.shouldSkipProxyHost("192.168.1.100") { + t.Error("expected host with active cooldown to return true") + } + + // Verify the host is still in the map + tc.proxyMu.Lock() + state, exists := tc.proxyHostStates["192.168.1.100"] + tc.proxyMu.Unlock() + + if !exists { + t.Error("expected host with active cooldown to remain in map") + } + if state.lastError != "connection refused" { + t.Errorf("expected lastError to be preserved, got %q", state.lastError) + } +} + +func TestShouldSkipProxyHost_ExpiredCooldownResetsState(t *testing.T) { + initialState := &proxyHostState{ + failures: 5, + cooldownUntil: time.Now().Add(-time.Second), // just expired + lastError: "previous error", + } + tc := &TemperatureCollector{ + proxyHostStates: map[string]*proxyHostState{ + "192.168.1.100": initialState, + }, + } + + // This call should reset the state and delete the host + result := tc.shouldSkipProxyHost("192.168.1.100") + if result { + t.Error("expected expired cooldown to return false") + } + + // After the call, the host should be deleted from the map + tc.proxyMu.Lock() + _, exists := tc.proxyHostStates["192.168.1.100"] + tc.proxyMu.Unlock() + + if exists { + t.Error("expected host to be deleted after cooldown expired") + } +} + +func TestShouldSkipProxyHost_TrimsWhitespace(t *testing.T) { + tc := &TemperatureCollector{ + proxyHostStates: map[string]*proxyHostState{ + "192.168.1.100": { + cooldownUntil: time.Now().Add(5 * time.Minute), + }, + }, + } + + // Host with leading/trailing whitespace should match after trimming + if !tc.shouldSkipProxyHost(" 192.168.1.100 ") { + t.Error("expected trimmed host to match entry in map") + } +} + // Helper functions for test setup func intPtr(i int) *int {