diff --git a/cmd/pulse-agent/main_test.go b/cmd/pulse-agent/main_test.go index b228e623d..63b8853c4 100644 --- a/cmd/pulse-agent/main_test.go +++ b/cmd/pulse-agent/main_test.go @@ -1275,7 +1275,7 @@ func TestRun(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - run(ctx, []string{"-token", "T", "-enable-host=false"}, func(s string) string { return "" }) + _ = run(ctx, []string{"-token", "T", "-enable-host=false"}, func(s string) string { return "" }) }) t.Run("auto-detect podman", func(t *testing.T) { @@ -1290,7 +1290,7 @@ func TestRun(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - run(ctx, []string{"-token", "T", "-enable-host=false"}, func(s string) string { return "" }) + _ = run(ctx, []string{"-token", "T", "-enable-host=false"}, func(s string) string { return "" }) }) t.Run("goroutine error", func(t *testing.T) { diff --git a/cmd/pulse/auto_import_test.go b/cmd/pulse/auto_import_test.go index 119ea8900..2c0c25d12 100644 --- a/cmd/pulse/auto_import_test.go +++ b/cmd/pulse/auto_import_test.go @@ -136,7 +136,9 @@ func TestPerformAutoImport_FileNormalizeError(t *testing.T) { t.Setenv("PULSE_INIT_CONFIG_PASSPHRASE", "pass") importFile := filepath.Join(dir, "empty.enc") - os.WriteFile(importFile, []byte(" "), 0600) + if err := os.WriteFile(importFile, []byte(" "), 0600); err != nil { + t.Fatalf("write import file: %v", err) + } t.Setenv("PULSE_INIT_CONFIG_FILE", importFile) if err := server.PerformAutoImport(); err == nil { diff --git a/internal/ai/demo.go b/internal/ai/demo.go index b35aa1d3f..de55863eb 100644 --- a/internal/ai/demo.go +++ b/internal/ai/demo.go @@ -154,10 +154,10 @@ func (p *PatrolService) InjectDemoFindings() { ResourceType: "app-container", Node: "docker-host-1", Evidence: "prior_digest=sha256:abc123def456 new_digest=sha256:789xyz012uvw restart_count=2", - DetectedAt: now.Add(-3 * time.Minute), - LastSeenAt: now.Add(-1 * time.Minute), - TimesRaised: 2, - Source: updateSafetySource, + DetectedAt: now.Add(-3 * time.Minute), + LastSeenAt: now.Add(-1 * time.Minute), + TimesRaised: 2, + Source: updateSafetySource, }, { ID: "demo-pdm-alert-node-offline", diff --git a/internal/ai/findings_pdm_alert_bridge_test.go b/internal/ai/findings_pdm_alert_bridge_test.go index 006aee7e9..0a1ffc81d 100644 --- a/internal/ai/findings_pdm_alert_bridge_test.go +++ b/internal/ai/findings_pdm_alert_bridge_test.go @@ -49,9 +49,9 @@ func TestPDMAlertBridge_NilSourceIsNoOp(t *testing.T) { // TestPDMAlertBridge_SeedOfflineOnlineCycle drives the bridge through three // snapshots: -// 1. seed -- node online, first observation, nothing emitted. -// 2. offline -- node transitions to offline, one finding emitted. -// 3. online -- node returns to online, one resolve sentinel emitted. +// 1. seed -- node online, first observation, nothing emitted. +// 2. offline -- node transitions to offline, one finding emitted. +// 3. online -- node returns to online, one resolve sentinel emitted. func TestPDMAlertBridge_SeedOfflineOnlineCycle(t *testing.T) { src := &fakePDMSource{ snapshots: [][]pdmResource{ diff --git a/internal/ai/findings_remind_persistence_test.go b/internal/ai/findings_remind_persistence_test.go index a29071649..1c2b4825e 100644 --- a/internal/ai/findings_remind_persistence_test.go +++ b/internal/ai/findings_remind_persistence_test.go @@ -30,7 +30,7 @@ func TestFindingsPersistenceAdapter_PreservesRemindAt(t *testing.T) { ResourceType: "storage", Node: "node1", Title: "Storage growth trending toward full", - Description: "Pool tank approaching threshold", + Description: "Pool tank approaching threshold", Source: "ai-analysis", DetectedAt: now.Add(-2 * time.Hour), LastSeenAt: now, diff --git a/internal/ai/findings_update_safety_emit_test.go b/internal/ai/findings_update_safety_emit_test.go index c2d25c127..5190f7ad3 100644 --- a/internal/ai/findings_update_safety_emit_test.go +++ b/internal/ai/findings_update_safety_emit_test.go @@ -14,7 +14,8 @@ import ( // Cycle 1: baseline -- no findings emitted. // Cycle 2: digest changed -- watcher emits one finding; store accepts it. // Cycle 3: same digest, window elapsed, no restarts -- watcher returns a -// resolve sentinel; ResolveWithReason marks the finding AutoResolved. +// +// resolve sentinel; ResolveWithReason marks the finding AutoResolved. func TestUpdateSafetyEmit_FullCycle(t *testing.T) { store := NewFindingsStore() w := newUpdateSafetyWatcher() diff --git a/internal/api/ai_handlers_cost_reset_additional_test.go b/internal/api/ai_handlers_cost_reset_additional_test.go index 76c028308..b9fb9b303 100644 --- a/internal/api/ai_handlers_cost_reset_additional_test.go +++ b/internal/api/ai_handlers_cost_reset_additional_test.go @@ -162,7 +162,7 @@ func TestHandleResetAICostHistory_BackupRenameFail(t *testing.T) { t.Fatalf("chmod: %v", err) } t.Cleanup(func() { - os.Chmod(tmp, 0755) // restore so TempDir cleanup works + _ = os.Chmod(tmp, 0755) // restore so TempDir cleanup works }) handler := newTestAISettingsHandler(cfg, persistence, nil) diff --git a/internal/api/config_handlers_cluster_additional_test.go b/internal/api/config_handlers_cluster_additional_test.go index 7b1be84fb..4b10f3277 100644 --- a/internal/api/config_handlers_cluster_additional_test.go +++ b/internal/api/config_handlers_cluster_additional_test.go @@ -455,10 +455,14 @@ func TestHandleGetMockMode(t *testing.T) { prevEnabled := mock.IsMockEnabled() t.Cleanup(func() { mock.SetMockConfig(prevConfig) - mock.SetEnabled(prevEnabled) + if err := mock.SetEnabled(prevEnabled); err != nil { + t.Errorf("restore mock mode: %v", err) + } }) - mock.SetEnabled(false) + if err := mock.SetEnabled(false); err != nil { + t.Fatalf("disable mock mode: %v", err) + } mock.SetMockConfig(mock.MockConfig{ NodeCount: 3, RandomMetrics: false, @@ -491,7 +495,9 @@ func TestHandleGetMockMode(t *testing.T) { func TestHandleUpdateMockMode_InvokesChangeHook(t *testing.T) { prevEnabled := mock.IsMockEnabled() t.Cleanup(func() { - mock.SetEnabled(prevEnabled) + if err := mock.SetEnabled(prevEnabled); err != nil { + t.Errorf("restore mock mode: %v", err) + } }) handler := newTestConfigHandlers(t, &config.Config{DataPath: t.TempDir()}) diff --git a/internal/api/config_node_handlers_additional_test.go b/internal/api/config_node_handlers_additional_test.go index d77a7f7a1..604c25650 100644 --- a/internal/api/config_node_handlers_additional_test.go +++ b/internal/api/config_node_handlers_additional_test.go @@ -8,7 +8,6 @@ import ( "strings" "testing" - "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" @@ -17,8 +16,7 @@ import ( // TestHandleGetNodes_MockMode_UsesReadState verifies that the mock-mode branch // of handleGetNodes uses ReadState typed accessors instead of GetState(). func TestHandleGetNodes_MockMode_UsesReadState(t *testing.T) { - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + setMockModeForTest(t, true) // Create a monitor with a resource store populated via ReadState. monitor := &monitoring.Monitor{} @@ -107,8 +105,7 @@ func TestHandleGetNodes_MockMode_UsesReadState(t *testing.T) { // (no resource store wired), the handler returns an empty node list without // calling GetState(). This replaced the former GetState() fallback test. func TestHandleGetNodes_MockMode_NilReadState(t *testing.T) { - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + setMockModeForTest(t, true) // Monitor with state but no resource store — GetUnifiedReadState() returns nil. monitor := &monitoring.Monitor{} @@ -157,8 +154,7 @@ func TestConfigHandlers_getMonitor_Legacy(t *testing.T) { // and the ReadState adapter with different names. Since the handler uses ReadState // exclusively (no GetState fallback), only ReadState names should appear. func TestHandleGetNodes_MockMode_ReadStateTakesPriority(t *testing.T) { - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + setMockModeForTest(t, true) monitor := &monitoring.Monitor{} diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 92372ae67..e238674bc 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -2004,9 +2004,7 @@ func TestContract_InfrastructureChartsHonorExplicitMetricFilters(t *testing.T) { } func TestContract_MockChartRoutesUseCanonicalMockUnifiedReadStateForVMwareHosts(t *testing.T) { - prevMock := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(prevMock) }) + setMockModeForTest(t, true) fixtures := vmware.DefaultFixtures() if len(fixtures.Hosts) == 0 { @@ -2472,19 +2470,16 @@ func TestContract_GenerateStyledMockSeries_UsesTimestampBasedCurve(t *testing.T) } func TestContract_PlatformMockToggleRebindsRuntimeConnectionsAndResources(t *testing.T) { - t.Setenv("PULSE_MOCK_MODE", "false") - prevMock := mock.IsMockEnabled() - mock.SetEnabled(false) - t.Cleanup(func() { - mock.SetEnabled(prevMock) - }) + setMockModeForTest(t, false) cfg := &config.Config{DataPath: t.TempDir()} monitor, err := monitoring.New(cfg) if err != nil { t.Fatalf("new monitor: %v", err) } - monitor.SetMockMode(false) + if err := monitor.SetMockMode(false); err != nil { + t.Fatalf("set monitor mock mode: %v", err) + } router := NewRouter(cfg, monitor, nil, nil, nil, "1.0.0") t.Cleanup(func() { @@ -2582,11 +2577,7 @@ func TestContract_PlatformMockConnectionListsUseSharedFixtureMetadata(t *testing setTrueNASFeatureForTest(t, true) setVMwareFeatureForTest(t, true) - prevMock := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { - mock.SetEnabled(prevMock) - }) + setMockModeForTest(t, true) t.Run("truenas", func(t *testing.T) { fixture := mock.DefaultTrueNASConnectionFixture() @@ -6274,10 +6265,14 @@ func TestContract_RecoveryPointsMockPathReturnsCanonicalProviderBackedFixtures(t previousEnabled := mock.IsMockEnabled() previousConfig := mock.GetConfig() t.Cleanup(func() { - mock.SetEnabled(false) + if err := mock.SetEnabled(false); err != nil { + t.Errorf("disable mock mode: %v", err) + } mock.SetMockConfig(previousConfig) if previousEnabled { - mock.SetEnabled(true) + if err := mock.SetEnabled(true); err != nil { + t.Errorf("restore mock mode: %v", err) + } mock.SetMockConfig(previousConfig) } }) @@ -6293,8 +6288,12 @@ func TestContract_RecoveryPointsMockPathReturnsCanonicalProviderBackedFixtures(t t.Setenv("PULSE_MOCK_K8S_PODS", "0") t.Setenv("PULSE_MOCK_K8S_DEPLOYMENTS", "0") - mock.SetEnabled(false) - mock.SetEnabled(true) + if err := mock.SetEnabled(false); err != nil { + t.Fatalf("disable mock mode: %v", err) + } + if err := mock.SetEnabled(true); err != nil { + t.Fatalf("enable mock mode: %v", err) + } req := httptest.NewRequest(http.MethodGet, "/api/recovery/points?platform=truenas&limit=10", nil) rec := httptest.NewRecorder() @@ -7096,7 +7095,7 @@ func TestContract_HostReportAdmissionPreservesRestartContinuityAtLimit(t *testin func TestContract_PlatformConnectionWritesIgnoreUsageUnavailableWithCapsRetired(t *testing.T) { t.Run("truenas add", func(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, _, monitor := newTrueNASHandlersForTest(t, nil) @@ -7123,7 +7122,7 @@ func TestContract_PlatformConnectionWritesIgnoreUsageUnavailableWithCapsRetired( t.Run("vmware add", func(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, _ := newVMwareHandlersForTest(t) @@ -7166,7 +7165,7 @@ func TestContract_PlatformConnectionWritesIgnoreUsageUnavailableWithCapsRetired( func TestContract_DisabledPlatformConnectionWritesBypassUnavailableUsageGate(t *testing.T) { t.Run("truenas add", func(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, _, monitor := newTrueNASHandlersForTest(t, nil) @@ -7194,7 +7193,7 @@ func TestContract_DisabledPlatformConnectionWritesBypassUnavailableUsageGate(t * t.Run("truenas update", func(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -7233,7 +7232,7 @@ func TestContract_DisabledPlatformConnectionWritesBypassUnavailableUsageGate(t * t.Run("vmware add", func(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, _ := newVMwareHandlersForTest(t) @@ -7274,7 +7273,7 @@ func TestContract_DisabledPlatformConnectionWritesBypassUnavailableUsageGate(t * t.Run("vmware update", func(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -11702,9 +11701,7 @@ func TestContract_ResourceListUsesDeterministicNameTieBreakers(t *testing.T) { } func TestContract_StateAndResourceListShareCanonicalMockResourceContract(t *testing.T) { - t.Setenv("PULSE_MOCK_MODE", "true") - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + setMockModeForTest(t, true) dataPath := t.TempDir() hashedPassword, err := authpkg.HashPassword("password") diff --git a/internal/api/maintenance_verification.go b/internal/api/maintenance_verification.go index b36306f27..8711dd856 100644 --- a/internal/api/maintenance_verification.go +++ b/internal/api/maintenance_verification.go @@ -20,36 +20,36 @@ import ( // Verification Report" name; the underlying loop substrate is // implementation detail and is not exposed verbatim. type maintenanceVerificationReportAPI struct { - ID string `json:"id"` - ResourceID string `json:"resourceId"` - Trigger string `json:"trigger"` - Goal string `json:"goal,omitempty"` - Status string `json:"status"` - StartedAt time.Time `json:"startedAt"` - CompletedAt time.Time `json:"completedAt"` - WindowStartedAt *time.Time `json:"windowStartedAt,omitempty"` - WindowEndedAt *time.Time `json:"windowEndedAt,omitempty"` - Evidence maintenanceVerificationEvidenceAPI `json:"evidence"` - LinkedFindingIDs []string `json:"linkedFindingIds"` - LinkedAlertIDs []string `json:"linkedAlertIds"` - LinkedActionIDs []string `json:"linkedActionIds"` - LinkedPatrolRunID string `json:"linkedPatrolRunId,omitempty"` - Recommendation string `json:"recommendation,omitempty"` - UserOutcome string `json:"userOutcome,omitempty"` - ReviewedAt *time.Time `json:"reviewedAt,omitempty"` - ReviewedBy string `json:"reviewedBy,omitempty"` - ReviewNote string `json:"reviewNote,omitempty"` + ID string `json:"id"` + ResourceID string `json:"resourceId"` + Trigger string `json:"trigger"` + Goal string `json:"goal,omitempty"` + Status string `json:"status"` + StartedAt time.Time `json:"startedAt"` + CompletedAt time.Time `json:"completedAt"` + WindowStartedAt *time.Time `json:"windowStartedAt,omitempty"` + WindowEndedAt *time.Time `json:"windowEndedAt,omitempty"` + Evidence maintenanceVerificationEvidenceAPI `json:"evidence"` + LinkedFindingIDs []string `json:"linkedFindingIds"` + LinkedAlertIDs []string `json:"linkedAlertIds"` + LinkedActionIDs []string `json:"linkedActionIds"` + LinkedPatrolRunID string `json:"linkedPatrolRunId,omitempty"` + Recommendation string `json:"recommendation,omitempty"` + UserOutcome string `json:"userOutcome,omitempty"` + ReviewedAt *time.Time `json:"reviewedAt,omitempty"` + ReviewedBy string `json:"reviewedBy,omitempty"` + ReviewNote string `json:"reviewNote,omitempty"` } type maintenanceVerificationEvidenceAPI struct { - OperatorStateSummary string `json:"operatorStateSummary,omitempty"` - ActiveCriticalAlerts int `json:"activeCriticalAlerts"` - ActiveWarningAlerts int `json:"activeWarningAlerts"` - ActiveCriticalFindings int `json:"activeCriticalFindings"` - ActiveWarningFindings int `json:"activeWarningFindings"` - FailedActionsSinceWindowStart int `json:"failedActionsSinceWindowStart"` - MetricRecovery *maintenanceVerificationMetricRecoveryAPI `json:"metricRecovery,omitempty"` - PatrolRunTODO string `json:"patrolRunTodo,omitempty"` + OperatorStateSummary string `json:"operatorStateSummary,omitempty"` + ActiveCriticalAlerts int `json:"activeCriticalAlerts"` + ActiveWarningAlerts int `json:"activeWarningAlerts"` + ActiveCriticalFindings int `json:"activeCriticalFindings"` + ActiveWarningFindings int `json:"activeWarningFindings"` + FailedActionsSinceWindowStart int `json:"failedActionsSinceWindowStart"` + MetricRecovery *maintenanceVerificationMetricRecoveryAPI `json:"metricRecovery,omitempty"` + PatrolRunTODO string `json:"patrolRunTodo,omitempty"` } type maintenanceVerificationMetricRecoveryAPI struct { diff --git a/internal/api/metrics_history_fallback_test.go b/internal/api/metrics_history_fallback_test.go index c0c3c2b74..1dd102fe9 100644 --- a/internal/api/metrics_history_fallback_test.go +++ b/internal/api/metrics_history_fallback_test.go @@ -88,8 +88,7 @@ func TestMetricsHistoryFallbackUsesLivePoint(t *testing.T) { } func TestMetricsHistoryFallbackMockDiskSynthesizesSeries(t *testing.T) { - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + setMockModeForTest(t, true) state := mock.CurrentFixtureGraph().State var disk models.PhysicalDisk diff --git a/internal/api/monitored_system_ledger_test.go b/internal/api/monitored_system_ledger_test.go index 05a6b0c1a..9843dc91e 100644 --- a/internal/api/monitored_system_ledger_test.go +++ b/internal/api/monitored_system_ledger_test.go @@ -227,7 +227,9 @@ func TestMonitoredSystemLedgerNilSystemsBecomesEmptyArray(t *testing.T) { t.Fatalf("marshal: %v", err) } var decoded map[string]interface{} - json.Unmarshal(data, &decoded) + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } systems, ok := decoded["systems"].([]interface{}) if !ok { t.Fatalf("systems is not an array: %T", decoded["systems"]) diff --git a/internal/api/recovery_handlers_test.go b/internal/api/recovery_handlers_test.go index aeeb7dbaa..9ebc03960 100644 --- a/internal/api/recovery_handlers_test.go +++ b/internal/api/recovery_handlers_test.go @@ -12,7 +12,6 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" - "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/recovery" recoverymanager "github.com/rcourtman/pulse-go-rewrite/internal/recovery/manager" _ "modernc.org/sqlite" @@ -101,11 +100,7 @@ func TestParseRecoveryItemResourceIDQuery(t *testing.T) { } func TestHandleListPointsAcceptsCanonicalPlatformQuery(t *testing.T) { - prevMock := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { - mock.SetEnabled(prevMock) - }) + setMockModeForTest(t, true) req := httptest.NewRequest(http.MethodGet, "/api/recovery/points?platform=truenas&limit=500", nil) rec := httptest.NewRecorder() @@ -178,11 +173,7 @@ func TestBuildRecoveryPointPayloadExposesCanonicalItemRefField(t *testing.T) { } func TestHandleListRollupsExposeCanonicalPlatformsPayload(t *testing.T) { - prevMock := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { - mock.SetEnabled(prevMock) - }) + setMockModeForTest(t, true) req := httptest.NewRequest(http.MethodGet, "/api/recovery/rollups?platform=truenas&limit=500", nil) rec := httptest.NewRecorder() diff --git a/internal/api/router_integration_test.go b/internal/api/router_integration_test.go index 0f285ff38..745585dcd 100644 --- a/internal/api/router_integration_test.go +++ b/internal/api/router_integration_test.go @@ -114,7 +114,9 @@ func newIntegrationServerWithRuntimeMode( if err != nil { t.Fatalf("failed to create monitor: %v", err) } - monitor.SetMockMode(mockMode) + if err := monitor.SetMockMode(mockMode); err != nil { + t.Fatalf("failed to set monitor mock mode: %v", err) + } hub.SetStateGetter(func(orgID string) interface{} { return monitor.BuildFrontendState() diff --git a/internal/api/router_misc_additional_test.go b/internal/api/router_misc_additional_test.go index 3703f3aa5..4ee7b8141 100644 --- a/internal/api/router_misc_additional_test.go +++ b/internal/api/router_misc_additional_test.go @@ -11,7 +11,6 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/ai" - "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" @@ -512,9 +511,7 @@ func TestHandleCharts_StatsDebugMetadata(t *testing.T) { } func TestHandleCharts_UsesCanonicalMockUnifiedReadStateForVMwareHosts(t *testing.T) { - prevMock := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(prevMock) }) + setMockModeForTest(t, true) fixtures := vmware.DefaultFixtures() if len(fixtures.Hosts) == 0 { @@ -707,9 +704,7 @@ func TestHandleInfrastructureCharts_MetricFilter(t *testing.T) { } func TestHandleInfrastructureCharts_UsesCanonicalMockUnifiedReadStateForVMwareHosts(t *testing.T) { - prevMock := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(prevMock) }) + setMockModeForTest(t, true) fixtures := vmware.DefaultFixtures() if len(fixtures.Hosts) == 0 { @@ -1210,9 +1205,7 @@ func TestHandleWorkloadsSummaryCharts_UsesCanonicalWorkloadIDsForVMwareVMs(t *te } func TestHandleWorkloadCharts_IncludesKubernetesPods(t *testing.T) { - prevMock := mock.IsMockEnabled() - mock.SetEnabled(false) - t.Cleanup(func() { mock.SetEnabled(prevMock) }) + setMockModeForTest(t, false) monitor, state, _ := newTestMonitor(t) state.Nodes = []models.Node{{ @@ -1279,9 +1272,7 @@ func TestHandleWorkloadCharts_IncludesKubernetesPods(t *testing.T) { } func TestHandleWorkloadsSummaryCharts_IncludesKubernetesPods(t *testing.T) { - prevMock := mock.IsMockEnabled() - mock.SetEnabled(false) - t.Cleanup(func() { mock.SetEnabled(prevMock) }) + setMockModeForTest(t, false) monitor, state, _ := newTestMonitor(t) state.KubernetesClusters = []models.KubernetesCluster{{ diff --git a/internal/api/router_mock_platforms_test.go b/internal/api/router_mock_platforms_test.go index 2edbfe836..64738e997 100644 --- a/internal/api/router_mock_platforms_test.go +++ b/internal/api/router_mock_platforms_test.go @@ -7,18 +7,13 @@ import ( "testing" "github.com/rcourtman/pulse-go-rewrite/internal/config" - "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/truenas" unified "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/internal/vmware" ) func TestRouterMockMode_SeedsTrueNASAndVMwareSupplementalResources(t *testing.T) { - previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { - mock.SetEnabled(previous) - }) + setMockModeForTest(t, true) cfg := &config.Config{DataPath: t.TempDir()} router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") @@ -71,11 +66,7 @@ func TestRouterMockMode_SeedsTrueNASAndVMwareSupplementalResources(t *testing.T) } func TestRouterMockMode_SeedsVMwareSupplementalActivity(t *testing.T) { - previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { - mock.SetEnabled(previous) - }) + setMockModeForTest(t, true) adapter := mockSupplementalRecordsAdapter{source: unified.SourceVMware} changes := adapter.SupplementalChanges(nil, "default") diff --git a/internal/api/system_settings_telemetry_test.go b/internal/api/system_settings_telemetry_test.go index 2fe0539f7..5b1c43f02 100644 --- a/internal/api/system_settings_telemetry_test.go +++ b/internal/api/system_settings_telemetry_test.go @@ -308,8 +308,8 @@ func TestTelemetryUpdate_NoMutationOnPersistFailure(t *testing.T) { t.Fatal(err) } t.Cleanup(func() { - os.Chmod(tempDir, 0700) - os.Chmod(systemFile, 0600) + _ = os.Chmod(tempDir, 0700) // restore so TempDir cleanup works + _ = os.Chmod(systemFile, 0600) // restore so TempDir cleanup works }) body, _ := json.Marshal(map[string]interface{}{"telemetryEnabled": false}) diff --git a/internal/api/truenas_handlers_test.go b/internal/api/truenas_handlers_test.go index b225c84db..2ed06332e 100644 --- a/internal/api/truenas_handlers_test.go +++ b/internal/api/truenas_handlers_test.go @@ -14,7 +14,6 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" - "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/truenas" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" @@ -35,7 +34,7 @@ func (c *fakeTrueNASClient) Close() {} func TestTrueNASHandlers_HandleAdd_Success(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) handler, persistence, _ := newTrueNASHandlersForTest(t, nil) @@ -79,7 +78,7 @@ func TestTrueNASHandlers_HandleAdd_Success(t *testing.T) { func TestTrueNASHandlers_HandleAdd_ValidationAndFeatureGate(t *testing.T) { t.Run("missing host", func(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) handler, _, _ := newTrueNASHandlersForTest(t, nil) body := marshalTrueNASRequest(t, map[string]any{ @@ -96,7 +95,7 @@ func TestTrueNASHandlers_HandleAdd_ValidationAndFeatureGate(t *testing.T) { t.Run("feature disabled", func(t *testing.T) { setTrueNASFeatureForTest(t, false) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) handler, _, _ := newTrueNASHandlersForTest(t, nil) body := marshalTrueNASRequest(t, map[string]any{ @@ -118,7 +117,7 @@ func TestTrueNASHandlers_HandleAdd_ValidationAndFeatureGate(t *testing.T) { func TestTrueNASHandlers_HandleAdd_BlocksNewCountedSystemAtLimit(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -188,7 +187,7 @@ func TestTrueNASHandlers_HandleAdd_ReturnsUnavailableWhenSupplementalInventoryNo } { t.Run(tc.name, func(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -220,7 +219,7 @@ func TestTrueNASHandlers_HandleAdd_ReturnsUnavailableWhenSupplementalInventoryNo func TestTrueNASHandlers_HandleAdd_DoesNotCountDisabledConnectionAtLimit(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -275,7 +274,7 @@ func TestTrueNASHandlers_HandleAdd_DoesNotCountDisabledConnectionAtLimit(t *test func TestTrueNASHandlers_HandleAdd_AllowsDisabledConnectionWhenUsageUnavailable(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -311,7 +310,7 @@ func TestTrueNASHandlers_HandleAdd_AllowsDisabledConnectionWhenUsageUnavailable( func TestTrueNASHandlers_HandleAdd_AllowsCanonicalOverlapAtLimit(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -412,7 +411,7 @@ func TestTrueNASHandlers_HandleList_RedactsSensitiveFields(t *testing.T) { func TestTrueNASHandlers_HandleList_ReturnsMockConnectionsInMockMode(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, true) + setMockModeForTest(t, true) handler, _, _ := newTrueNASHandlersForTest(t, nil) @@ -525,7 +524,7 @@ func TestTrueNASHandlers_HandleList_IncludesPollAndObservedSummary(t *testing.T) func TestTrueNASHandlers_HandleDelete_RemovesAndHandlesUnknownID(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) handler, persistence, _ := newTrueNASHandlersForTest(t, nil) if err := persistence.SaveTrueNASConfig([]config.TrueNASInstance{ @@ -562,7 +561,7 @@ func TestTrueNASHandlers_HandleDelete_RemovesAndHandlesUnknownID(t *testing.T) { func TestTrueNASHandlers_HandleUpdate_PreservesMaskedSecretsAndReplacesFields(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) handler, persistence, _ := newTrueNASHandlersForTest(t, nil) if err := persistence.SaveTrueNASConfig([]config.TrueNASInstance{ @@ -630,7 +629,7 @@ func TestTrueNASHandlers_HandleUpdate_PreservesMaskedSecretsAndReplacesFields(t func TestTrueNASHandlers_HandleUpdate_BlocksProjectedNetNewSystemAtLimit(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -728,7 +727,7 @@ func TestTrueNASHandlers_HandleUpdate_ReturnsUnavailableWhenSupplementalInventor } { t.Run(tc.name, func(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -774,7 +773,7 @@ func TestTrueNASHandlers_HandleUpdate_ReturnsUnavailableWhenSupplementalInventor func TestTrueNASHandlers_HandleUpdate_AllowsDisablingConnectionWhenUsageUnavailable(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence, monitor := newTrueNASHandlersForTest(t, nil) @@ -829,7 +828,7 @@ func TestTrueNASHandlers_HandleUpdate_AllowsDisablingConnectionWhenUsageUnavaila func TestTrueNASHandlers_HandleUpdate_UnknownID(t *testing.T) { setTrueNASFeatureForTest(t, true) - setMockModeForTrueNASTest(t, false) + setMockModeForTest(t, false) handler, _, _ := newTrueNASHandlersForTest(t, nil) @@ -1447,15 +1446,6 @@ func setTrueNASFeatureForTest(t *testing.T, enabled bool) { }) } -func setMockModeForTrueNASTest(t *testing.T, enabled bool) { - t.Helper() - previous := mock.IsMockEnabled() - mock.SetEnabled(enabled) - t.Cleanup(func() { - mock.SetEnabled(previous) - }) -} - func marshalTrueNASRequest(t *testing.T, payload map[string]any) []byte { t.Helper() body, err := json.Marshal(payload) diff --git a/internal/api/vmware_handlers_test.go b/internal/api/vmware_handlers_test.go index 3432d8e7d..ac8eece8a 100644 --- a/internal/api/vmware_handlers_test.go +++ b/internal/api/vmware_handlers_test.go @@ -12,7 +12,6 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" - "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" @@ -34,7 +33,7 @@ func (c *fakeVMwareClient) Close() {} func TestVMwareHandlers_HandleAdd_Success(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) handler, persistence := newVMwareHandlersForTest(t) @@ -81,7 +80,7 @@ func TestVMwareHandlers_HandleAdd_Success(t *testing.T) { func TestVMwareHandlers_HandleAdd_ValidationAndFeatureGate(t *testing.T) { t.Run("missing host", func(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) handler, _ := newVMwareHandlersForTest(t) body := marshalVMwareRequest(t, map[string]any{ @@ -99,7 +98,7 @@ func TestVMwareHandlers_HandleAdd_ValidationAndFeatureGate(t *testing.T) { t.Run("feature disabled", func(t *testing.T) { setVMwareFeatureForTest(t, false) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) handler, _ := newVMwareHandlersForTest(t) body := marshalVMwareRequest(t, map[string]any{ @@ -122,7 +121,7 @@ func TestVMwareHandlers_HandleAdd_ValidationAndFeatureGate(t *testing.T) { func TestVMwareHandlers_HandleAdd_BlocksProjectedNetNewSystemsAtLimit(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -216,7 +215,7 @@ func TestVMwareHandlers_HandleAdd_ReturnsUnavailableBeforePreviewingInventory(t } { t.Run(tc.name, func(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -262,7 +261,7 @@ func TestVMwareHandlers_HandleAdd_ReturnsUnavailableBeforePreviewingInventory(t func TestVMwareHandlers_HandleAdd_DoesNotCountDisabledConnectionAtLimit(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -328,7 +327,7 @@ func TestVMwareHandlers_HandleAdd_DoesNotCountDisabledConnectionAtLimit(t *testi func TestVMwareHandlers_HandleAdd_AllowsDisabledConnectionWhenUsageUnavailable(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -377,7 +376,7 @@ func TestVMwareHandlers_HandleAdd_AllowsDisabledConnectionWhenUsageUnavailable(t func TestVMwareHandlers_HandleAdd_AllowsCanonicalOverlapAtLimit(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -519,7 +518,7 @@ func TestVMwareHandlers_HandleList_RedactsSensitiveFieldsAndIncludesRuntimeSumma func TestVMwareHandlers_HandleList_ReturnsMockConnectionsInMockMode(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, true) + setMockModeForTest(t, true) handler, _ := newVMwareHandlersForTest(t) @@ -554,7 +553,7 @@ func TestVMwareHandlers_HandleList_ReturnsMockConnectionsInMockMode(t *testing.T func TestVMwareHandlers_HandleDelete_RemovesAndClearsRuntimeSummary(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) handler, persistence := newVMwareHandlersForTest(t) if err := persistence.SaveVMwareConfig([]config.VMwareVCenterInstance{ @@ -652,7 +651,7 @@ func TestVMwareHandlers_HandleList_CarriesDegradedObservedSummary(t *testing.T) func TestVMwareHandlers_HandleUpdate_PreservesMaskedSecretsAndReplacesFields(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) handler, persistence := newVMwareHandlersForTest(t) if err := persistence.SaveVMwareConfig([]config.VMwareVCenterInstance{ @@ -720,7 +719,7 @@ func TestVMwareHandlers_HandleUpdate_PreservesMaskedSecretsAndReplacesFields(t * func TestVMwareHandlers_HandleUpdate_BlocksProjectedNetNewSystemsAtLimit(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -851,7 +850,7 @@ func TestVMwareHandlers_HandleUpdate_ReturnsUnavailableBeforePreviewingInventory } { t.Run(tc.name, func(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -910,7 +909,7 @@ func TestVMwareHandlers_HandleUpdate_ReturnsUnavailableBeforePreviewingInventory func TestVMwareHandlers_HandleUpdate_AllowsDisablingConnectionWhenUsageUnavailable(t *testing.T) { setVMwareFeatureForTest(t, true) - setMockModeForVMwareTest(t, false) + setMockModeForTest(t, false) setMaxMonitoredSystemsLicenseForTests(t, 1) handler, persistence := newVMwareHandlersForTest(t) @@ -1696,15 +1695,6 @@ func setVMwareFeatureForTest(t *testing.T, enabled bool) { }) } -func setMockModeForVMwareTest(t *testing.T, enabled bool) { - t.Helper() - previous := mock.IsMockEnabled() - mock.SetEnabled(enabled) - t.Cleanup(func() { - mock.SetEnabled(previous) - }) -} - func marshalVMwareRequest(t *testing.T, payload map[string]any) []byte { t.Helper() body, err := json.Marshal(payload) diff --git a/internal/config/persistence_nodes_save_test.go b/internal/config/persistence_nodes_save_test.go index 8ae997053..0c0e6de1b 100644 --- a/internal/config/persistence_nodes_save_test.go +++ b/internal/config/persistence_nodes_save_test.go @@ -18,8 +18,14 @@ func TestSaveNodesConfig_Scenarios(t *testing.T) { // 1. Mock mode enabled t.Run("MockModeEnabled", func(t *testing.T) { - mock.SetEnabled(true) - defer mock.SetEnabled(false) + if err := mock.SetEnabled(true); err != nil { + t.Fatalf("enable mock mode: %v", err) + } + defer func() { + if err := mock.SetEnabled(false); err != nil { + t.Errorf("disable mock mode: %v", err) + } + }() err := cp.SaveNodesConfig([]PVEInstance{{Host: "test"}}, nil, nil) assert.NoError(t, err) diff --git a/internal/kubernetesagent/agent_inventory_test.go b/internal/kubernetesagent/agent_inventory_test.go index 74de9dc82..aacdd8724 100644 --- a/internal/kubernetesagent/agent_inventory_test.go +++ b/internal/kubernetesagent/agent_inventory_test.go @@ -77,7 +77,9 @@ func TestCollectNativePolicyConfigAndAutoscalingInventory(t *testing.T) { }) metadataScheme := metadatafake.NewTestScheme() - metav1.AddMetaToScheme(metadataScheme) + if err := metav1.AddMetaToScheme(metadataScheme); err != nil { + t.Fatalf("add meta to scheme: %v", err) + } metadataClient := metadatafake.NewSimpleMetadataClient( metadataScheme, &metav1.PartialObjectMetadata{ diff --git a/internal/mock/config_validation_test.go b/internal/mock/config_validation_test.go index f199099f3..592cc9d00 100644 --- a/internal/mock/config_validation_test.go +++ b/internal/mock/config_validation_test.go @@ -37,9 +37,9 @@ func TestLoadMockConfigRejectsInvalidValues(t *testing.T) { } func TestSetMockConfigNormalizesInvalidValues(t *testing.T) { - SetEnabled(false) + mustSetEnabled(t, false) t.Cleanup(func() { - SetEnabled(false) + mustSetEnabled(t, false) SetMockConfig(DefaultConfig) }) diff --git a/internal/mock/generator_test.go b/internal/mock/generator_test.go index 75d423f35..faa330d1b 100644 --- a/internal/mock/generator_test.go +++ b/internal/mock/generator_test.go @@ -498,9 +498,9 @@ func TestBuildFixtureStateIncludesKubernetesNetworkingInventory(t *testing.T) { } func TestMockStateIncludesHostAgents(t *testing.T) { - SetEnabled(true) + mustSetEnabled(t, true) t.Cleanup(func() { - SetEnabled(false) + mustSetEnabled(t, false) }) state := CurrentFixtureGraph().State diff --git a/internal/mock/integration_concurrency_test.go b/internal/mock/integration_concurrency_test.go index 4b04bcbea..3bdab263e 100644 --- a/internal/mock/integration_concurrency_test.go +++ b/internal/mock/integration_concurrency_test.go @@ -6,22 +6,24 @@ import ( ) func TestSetEnabledDisableDoesNotDeadlockWhenUpdateIsBlocked(t *testing.T) { - SetEnabled(false) + mustSetEnabled(t, false) t.Cleanup(func() { - SetEnabled(false) + mustSetEnabled(t, false) }) cfg := DefaultConfig cfg.RandomMetrics = true SetMockConfig(cfg) - SetEnabled(true) + mustSetEnabled(t, true) dataMu.Lock() time.Sleep(currentMockUpdateInterval() + 250*time.Millisecond) done := make(chan struct{}) go func() { - SetEnabled(false) + if err := SetEnabled(false); err != nil { + t.Errorf("SetEnabled(false): %v", err) + } close(done) }() diff --git a/internal/mock/integration_coverage_test.go b/internal/mock/integration_coverage_test.go index 7abc60a38..ada78ef1c 100644 --- a/internal/mock/integration_coverage_test.go +++ b/internal/mock/integration_coverage_test.go @@ -178,12 +178,16 @@ func TestSetEnabledNoOpBehaviorForInitAndRuntime(t *testing.T) { t.Fatalf("failed to seed env: %v", err) } - setEnabled(false, true) + if err := setEnabled(false, true); err != nil { + t.Fatalf("setEnabled: %v", err) + } if got := os.Getenv("PULSE_MOCK_MODE"); got != "preserve" { t.Fatalf("expected init no-op to preserve env, got %q", got) } - setEnabled(false, false) + if err := setEnabled(false, false); err != nil { + t.Fatalf("setEnabled: %v", err) + } if got := os.Getenv("PULSE_MOCK_MODE"); got != "false" { t.Fatalf("expected runtime no-op to write false env, got %q", got) } @@ -278,21 +282,21 @@ func TestSetEnabledUsesCurrentConfigInsteadOfReloadingEnv(t *testing.T) { prevCfg := GetConfig() t.Cleanup(func() { SetMockConfig(prevCfg) - SetEnabled(prevEnabled) + mustSetEnabled(t, prevEnabled) }) t.Setenv("PULSE_MOCK_NODES", "9") t.Setenv("PULSE_MOCK_VMS_PER_NODE", "7") t.Setenv("PULSE_MOCK_LXCS_PER_NODE", "6") - SetEnabled(false) + mustSetEnabled(t, false) custom := DefaultConfig custom.NodeCount = 2 custom.VMsPerNode = 1 custom.LXCsPerNode = 1 SetMockConfig(custom) - SetEnabled(true) + mustSetEnabled(t, true) got := GetConfig() if got.NodeCount != custom.NodeCount || got.VMsPerNode != custom.VMsPerNode || got.LXCsPerNode != custom.LXCsPerNode { diff --git a/internal/mock/integration_security_test.go b/internal/mock/integration_security_test.go index 82dee48f2..13f035470 100644 --- a/internal/mock/integration_security_test.go +++ b/internal/mock/integration_security_test.go @@ -14,10 +14,10 @@ func TestSetMockConfigNormalizesInvalidAndOversizedValues(t *testing.T) { prevEnabled := IsMockEnabled() t.Cleanup(func() { SetMockConfig(prevConfig) - SetEnabled(prevEnabled) + mustSetEnabled(t, prevEnabled) }) - SetEnabled(false) + mustSetEnabled(t, false) overlong := strings.Repeat("a", maxMockHighLoadNodeChars+1) SetMockConfig(MockConfig{ diff --git a/internal/mock/integration_shutdown_test.go b/internal/mock/integration_shutdown_test.go index f80499677..603995930 100644 --- a/internal/mock/integration_shutdown_test.go +++ b/internal/mock/integration_shutdown_test.go @@ -6,9 +6,9 @@ import ( ) func TestSetEnabledDisableDoesNotDeadlockWhenLoopNeedsStateLock(t *testing.T) { - SetEnabled(false) + mustSetEnabled(t, false) t.Cleanup(func() { - SetEnabled(false) + mustSetEnabled(t, false) }) dataMu.Lock() @@ -26,7 +26,9 @@ func TestSetEnabledDisableDoesNotDeadlockWhenLoopNeedsStateLock(t *testing.T) { done := make(chan struct{}) go func() { - SetEnabled(false) + if err := SetEnabled(false); err != nil { + t.Errorf("SetEnabled(false): %v", err) + } close(done) }() diff --git a/internal/mock/platform_fixtures_test.go b/internal/mock/platform_fixtures_test.go index 6287c87d6..b9783e710 100644 --- a/internal/mock/platform_fixtures_test.go +++ b/internal/mock/platform_fixtures_test.go @@ -12,8 +12,8 @@ import ( func TestUnifiedResourceSnapshotIncludesPlatformFixtures(t *testing.T) { previous := IsMockEnabled() - SetEnabled(true) - t.Cleanup(func() { SetEnabled(previous) }) + mustSetEnabled(t, true) + t.Cleanup(func() { mustSetEnabled(t, previous) }) graph := CurrentFixtureGraph() legacyName := "" @@ -72,8 +72,8 @@ func TestUnifiedResourceSnapshotIncludesPlatformFixtures(t *testing.T) { func TestUnifiedResourceSnapshotIncludesRuntimeNativeTabFixtures(t *testing.T) { previous := IsMockEnabled() - SetEnabled(true) - t.Cleanup(func() { SetEnabled(previous) }) + mustSetEnabled(t, true) + t.Cleanup(func() { mustSetEnabled(t, previous) }) resources, _ := UnifiedResourceSnapshot() counts := make(map[unifiedresources.ResourceType]int) @@ -110,8 +110,8 @@ func TestUnifiedResourceSnapshotIncludesRuntimeNativeTabFixtures(t *testing.T) { func TestUnifiedResourceSnapshotParentsDemoProxmoxWorkloads(t *testing.T) { previous := IsMockEnabled() - SetEnabled(true) - t.Cleanup(func() { SetEnabled(previous) }) + mustSetEnabled(t, true) + t.Cleanup(func() { mustSetEnabled(t, previous) }) resources, _ := UnifiedResourceSnapshot() agentsByID := make(map[string]unifiedresources.Resource) diff --git a/internal/mock/recovery_points_test.go b/internal/mock/recovery_points_test.go index ca0c15ff1..28589e011 100644 --- a/internal/mock/recovery_points_test.go +++ b/internal/mock/recovery_points_test.go @@ -10,8 +10,8 @@ import ( func TestCurrentFixtureGraphReturnsDefensiveCopies(t *testing.T) { previous := IsMockEnabled() - SetEnabled(true) - t.Cleanup(func() { SetEnabled(previous) }) + mustSetEnabled(t, true) + t.Cleanup(func() { mustSetEnabled(t, previous) }) graph := CurrentFixtureGraph() if len(graph.State.Nodes) == 0 { @@ -77,8 +77,8 @@ func TestCurrentFixtureGraphReturnsDefensiveCopies(t *testing.T) { func TestFixtureGraphRecoveryPointsDeriveSubjectsFromCurrentGraph(t *testing.T) { previous := IsMockEnabled() - SetEnabled(true) - t.Cleanup(func() { SetEnabled(previous) }) + mustSetEnabled(t, true) + t.Cleanup(func() { mustSetEnabled(t, previous) }) graph := CurrentFixtureGraph() if len(graph.State.KubernetesClusters) == 0 { @@ -161,8 +161,8 @@ func TestFixtureGraphRecoveryPointsDeriveSubjectsFromCurrentGraph(t *testing.T) func TestFixtureGraphRecoveryPointsKeepTrueNASArtifactsFreshForDemoMode(t *testing.T) { previous := IsMockEnabled() - SetEnabled(true) - t.Cleanup(func() { SetEnabled(previous) }) + mustSetEnabled(t, true) + t.Cleanup(func() { mustSetEnabled(t, previous) }) points := CurrentFixtureGraph().RecoveryPoints() if len(points) == 0 { @@ -189,8 +189,8 @@ func TestFixtureGraphRecoveryPointsKeepTrueNASArtifactsFreshForDemoMode(t *testi func TestFixtureGraphRecoveryPointsUseReadableKubernetesPVCUIDs(t *testing.T) { previous := IsMockEnabled() - SetEnabled(true) - t.Cleanup(func() { SetEnabled(previous) }) + mustSetEnabled(t, true) + t.Cleanup(func() { mustSetEnabled(t, previous) }) points := CurrentFixtureGraph().RecoveryPoints() if len(points) == 0 { diff --git a/internal/mock/testhelpers_test.go b/internal/mock/testhelpers_test.go new file mode 100644 index 000000000..3c8a1a037 --- /dev/null +++ b/internal/mock/testhelpers_test.go @@ -0,0 +1,12 @@ +package mock + +import "testing" + +// mustSetEnabled toggles the package mock-mode state and fails the test on +// error so callers don't repeat the check at every toggle. +func mustSetEnabled(t testing.TB, enabled bool) { + t.Helper() + if err := SetEnabled(enabled); err != nil { + t.Fatalf("SetEnabled(%v): %v", enabled, err) + } +} diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index b186e574c..a88529125 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -1072,8 +1072,8 @@ func TestMonitoringRuntimeAvoidsLegacyMockPartialHelpers(t *testing.T) { func TestMockOwnedUnifiedMetricSyncDefersToCanonicalSamplerInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(previous) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) if !shouldSkipMockOwnedUnifiedMetricSync(unifiedresources.Resource{ Sources: []unifiedresources.DataSource{unifiedresources.SourceTrueNAS}, @@ -1091,7 +1091,7 @@ func TestMockOwnedUnifiedMetricSyncDefersToCanonicalSamplerInMockMode(t *testing t.Fatal("expected all mock-owned resources to defer to the canonical mock sampler") } - mock.SetEnabled(false) + mustSetMockEnabled(t, false) if shouldSkipMockOwnedUnifiedMetricSync(unifiedresources.Resource{ Sources: []unifiedresources.DataSource{unifiedresources.SourceDocker}, }) { diff --git a/internal/monitoring/memory_trust_characterization_test.go b/internal/monitoring/memory_trust_characterization_test.go index c379ff703..b58bf6a1a 100644 --- a/internal/monitoring/memory_trust_characterization_test.go +++ b/internal/monitoring/memory_trust_characterization_test.go @@ -659,8 +659,8 @@ func TestPollVMsWithNodes_SkipsNativeGuestMetricWritesInMockMode(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(previous) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) mon := newTestPVEMonitor("test") defer mon.alertManager.Stop() @@ -707,8 +707,8 @@ func TestRecordGuestMetric_SkipsNativeWritesInMockMode(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(previous) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) mon := newTestPVEMonitor("test") defer mon.alertManager.Stop() diff --git a/internal/monitoring/mock_metrics_history_test.go b/internal/monitoring/mock_metrics_history_test.go index c2f88efd3..0bdb83cda 100644 --- a/internal/monitoring/mock_metrics_history_test.go +++ b/internal/monitoring/mock_metrics_history_test.go @@ -678,17 +678,17 @@ func TestSeedMockMetricsHistory_UsesCanonicalMockFixtureGraphForLegacyAndProvide previous := mock.IsMockEnabled() previousConfig := mock.GetConfig() t.Cleanup(func() { - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(previousConfig) if previous { - mock.SetEnabled(true) + mustSetMockEnabled(t, true) mock.SetMockConfig(previousConfig) } }) - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(compactMockFixtureConfig()) - mock.SetEnabled(true) + mustSetMockEnabled(t, true) graph := mock.CurrentFixtureGraph() if len(graph.State.Nodes) == 0 { @@ -700,7 +700,7 @@ func TestSeedMockMetricsHistory_UsesCanonicalMockFixtureGraphForLegacyAndProvide if len(graph.PlatformFixtures.VMware.Hosts) == 0 { t.Fatal("expected canonical mock graph to include VMware fixtures") } - mock.SetEnabled(false) + mustSetMockEnabled(t, false) now := time.Now() seedDuration := boundedMockHistoryProofWindow @@ -760,10 +760,10 @@ func TestStartMockMetricsSampler_DoesNotClearExistingMetricsStoreData(t *testing previousEnabled := mock.IsMockEnabled() previousConfig := mock.GetConfig() t.Cleanup(func() { - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(previousConfig) if previousEnabled { - mock.SetEnabled(true) + mustSetMockEnabled(t, true) mock.SetMockConfig(previousConfig) } }) @@ -793,9 +793,9 @@ func TestStartMockMetricsSampler_DoesNotClearExistingMetricsStoreData(t *testing }, }) - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(compactMockFixtureConfig()) - mock.SetEnabled(true) + mustSetMockEnabled(t, true) monitor := &Monitor{ metricsHistory: NewMetricsHistory(1000, 24*time.Hour), @@ -834,10 +834,10 @@ func TestStartAndStopMockMetricsSampler_ClearStaleMockChartCaches(t *testing.T) previousEnabled := mock.IsMockEnabled() previousConfig := mock.GetConfig() t.Cleanup(func() { - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(previousConfig) if previousEnabled { - mock.SetEnabled(true) + mustSetMockEnabled(t, true) mock.SetMockConfig(previousConfig) } }) @@ -854,9 +854,9 @@ func TestStartAndStopMockMetricsSampler_ClearStaleMockChartCaches(t *testing.T) cfg.K8sPodsPerCluster = 0 cfg.K8sDeploymentsPerCluster = 0 - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(cfg) - mock.SetEnabled(true) + mustSetMockEnabled(t, true) monitor := &Monitor{ metricsHistory: NewMetricsHistory(128, 24*time.Hour), @@ -915,19 +915,19 @@ func TestStartMockMetricsSampler_SeedsCanonicalMockResourceHistory(t *testing.T) previousEnabled := mock.IsMockEnabled() previousConfig := mock.GetConfig() t.Cleanup(func() { - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(previousConfig) if previousEnabled { - mock.SetEnabled(true) + mustSetMockEnabled(t, true) mock.SetMockConfig(previousConfig) } }) cfg := compactMockChartFixtureConfig() - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(cfg) - mock.SetEnabled(true) + mustSetMockEnabled(t, true) resources, _ := mock.UnifiedResourceSnapshot() if len(resources) == 0 { diff --git a/internal/monitoring/mock_mode_config_test.go b/internal/monitoring/mock_mode_config_test.go index 9b3035d04..b54ac015c 100644 --- a/internal/monitoring/mock_mode_config_test.go +++ b/internal/monitoring/mock_mode_config_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/rcourtman/pulse-go-rewrite/internal/config" - "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" ) @@ -60,8 +59,8 @@ func TestNew_MockModeClientInitializationRespectsKeepRealPollingSetting(t *testi t.Run("disabled_by_default", func(t *testing.T) { t.Setenv(mockKeepRealPollingEnv, "") - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, false) }) withClientStub(t, func(called *bool) { monitor, err := New(makeConfig(t.TempDir())) @@ -81,8 +80,8 @@ func TestNew_MockModeClientInitializationRespectsKeepRealPollingSetting(t *testi t.Run("disabled_via_env", func(t *testing.T) { t.Setenv(mockKeepRealPollingEnv, "false") - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, false) }) withClientStub(t, func(called *bool) { monitor, err := New(makeConfig(t.TempDir())) diff --git a/internal/monitoring/mock_mode_testhelpers_test.go b/internal/monitoring/mock_mode_testhelpers_test.go new file mode 100644 index 000000000..d1bfc3321 --- /dev/null +++ b/internal/monitoring/mock_mode_testhelpers_test.go @@ -0,0 +1,25 @@ +package monitoring + +import ( + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/mock" +) + +// mustSetMockEnabled toggles the package mock-mode state and fails the test +// on error so callers don't repeat the check at every toggle. +func mustSetMockEnabled(t testing.TB, enabled bool) { + t.Helper() + if err := mock.SetEnabled(enabled); err != nil { + t.Fatalf("mock.SetEnabled(%v): %v", enabled, err) + } +} + +// mustSetMonitorMockMode flips a monitor between mock and real data and fails +// the test on error. +func mustSetMonitorMockMode(t testing.TB, m *Monitor, enabled bool) { + t.Helper() + if err := m.SetMockMode(enabled); err != nil { + t.Fatalf("SetMockMode(%v): %v", enabled, err) + } +} diff --git a/internal/monitoring/monitor_docker_test.go b/internal/monitoring/monitor_docker_test.go index 66748665d..e06ffa4fe 100644 --- a/internal/monitoring/monitor_docker_test.go +++ b/internal/monitoring/monitor_docker_test.go @@ -196,8 +196,8 @@ func TestApplyDockerReportUsesTokenToDisambiguateAgentIDCollisions(t *testing.T) func TestApplyDockerReportSkipsMetricsHistoryInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(previous) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) monitor := newTestMonitor(t) report := agentsdocker.Report{ diff --git a/internal/monitoring/monitor_extra_coverage_test.go b/internal/monitoring/monitor_extra_coverage_test.go index daf044314..e02e280b5 100644 --- a/internal/monitoring/monitor_extra_coverage_test.go +++ b/internal/monitoring/monitor_extra_coverage_test.go @@ -30,8 +30,8 @@ func TestMonitor_GetConnectionStatuses_MockMode_Extra(t *testing.T) { } defer m.alertManager.Stop() - m.SetMockMode(true) - defer m.SetMockMode(false) + mustSetMonitorMockMode(t, m, true) + defer mustSetMonitorMockMode(t, m, false) statuses := m.GetConnectionStatuses() if statuses == nil { @@ -158,13 +158,13 @@ func TestMonitor_SetMockMode_Advanced_Extra(t *testing.T) { defer m.alertManager.Stop() // Switch to mock mode - m.SetMockMode(true) + mustSetMonitorMockMode(t, m, true) if !mock.IsMockEnabled() { t.Error("Mock mode should be enabled") } // Switch back - m.SetMockMode(false) + mustSetMonitorMockMode(t, m, false) if mock.IsMockEnabled() { t.Error("Mock mode should be disabled") } @@ -2359,8 +2359,8 @@ func TestMonitor_Start_Extra(t *testing.T) { defer m.alertManager.Stop() // Use MockMode to skip discovery - m.SetMockMode(true) - defer m.SetMockMode(false) + mustSetMonitorMockMode(t, m, true) + defer mustSetMonitorMockMode(t, m, false) m.mockMetricsCancel = func() {} // Skip mock metrics seeding to keep Start responsive in tests. ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index a5af907af..926836e64 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -1706,8 +1706,8 @@ func TestApplyHostReportPersistsPhysicalDiskIOMetricsForAgentDisks(t *testing.T) func TestApplyHostReportSkipsMetricsAndSMARTWritesInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(previous) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) storeCfg := metrics.DefaultConfig(t.TempDir()) storeCfg.WriteBufferSize = 1 diff --git a/internal/monitoring/monitor_metrics_chart_fallback_test.go b/internal/monitoring/monitor_metrics_chart_fallback_test.go index 5116dd055..b161a7b80 100644 --- a/internal/monitoring/monitor_metrics_chart_fallback_test.go +++ b/internal/monitoring/monitor_metrics_chart_fallback_test.go @@ -478,8 +478,8 @@ func TestGetPhysicalDiskTemperatureCharts_UsesNativeHistoryWhenStoreCoverageShal func TestGetPhysicalDiskTemperatureCharts_PrefersSeededMockHistoryInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) graph := mock.CurrentFixtureGraph() if len(graph.PlatformFixtures.TrueNAS.Disks) == 0 { @@ -532,8 +532,8 @@ func TestGetPhysicalDiskTemperatureCharts_PrefersSeededMockHistoryInMockMode(t * func TestGetGuestMetricsForChart_UsesCanonicalMockSamplerInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Second) @@ -550,8 +550,8 @@ func TestGetGuestMetricsForChart_UsesCanonicalMockSamplerInMockMode(t *testing.T func TestGetGuestMetricsForChart_PrefersSeededMockHistoryInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Second) @@ -579,8 +579,8 @@ func TestGetGuestMetricsForChart_PrefersSeededMockHistoryInMockMode(t *testing.T func TestGetGuestMetricsForChartBatch_PrefersSeededMockHistoryInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Second) @@ -611,8 +611,8 @@ func TestGetGuestMetricsForChartBatch_PrefersSeededMockHistoryInMockMode(t *test func TestGetGuestMetricsForChartBatch_DownsamplesDenseSeededMockHistory(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Minute) @@ -640,8 +640,8 @@ func TestGetGuestMetricsForChartBatch_DownsamplesDenseSeededMockHistory(t *testi func TestGetStorageMetricsForChart_UsesCanonicalMockSamplerInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) graph := mock.CurrentFixtureGraph() if len(graph.State.Storage) == 0 { @@ -670,8 +670,8 @@ func TestGetStorageMetricsForChart_UsesCanonicalMockSamplerInMockMode(t *testing func TestGetStorageMetricsForChart_PrefersSeededMockHistoryInMockMode(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Second) @@ -707,8 +707,8 @@ func TestGetStorageMetricsForChart_PrefersSeededMockHistoryInMockMode(t *testing func TestGetStorageMetricsForChart_DownsamplesDenseSeededMockHistory(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - defer mock.SetEnabled(previous) + mustSetMockEnabled(t, true) + defer mustSetMockEnabled(t, previous) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Minute) diff --git a/internal/monitoring/monitor_metrics_slo_test.go b/internal/monitoring/monitor_metrics_slo_test.go index d41e9b2ee..c743db80c 100644 --- a/internal/monitoring/monitor_metrics_slo_test.go +++ b/internal/monitoring/monitor_metrics_slo_test.go @@ -58,10 +58,10 @@ func TestStartMockMetricsSampler_PrewarmsDefaultWorkloadChartCaches(t *testing.T previousEnabled := mock.IsMockEnabled() previousConfig := mock.GetConfig() t.Cleanup(func() { - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(previousConfig) if previousEnabled { - mock.SetEnabled(true) + mustSetMockEnabled(t, true) mock.SetMockConfig(previousConfig) } }) @@ -75,9 +75,9 @@ func TestStartMockMetricsSampler_PrewarmsDefaultWorkloadChartCaches(t *testing.T cfg.K8sNodesPerCluster = 1 cfg.K8sPodsPerCluster = 1 - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(cfg) - mock.SetEnabled(true) + mustSetMockEnabled(t, true) monitor := &Monitor{ metricsHistory: NewMetricsHistory(128, 24*time.Hour), @@ -537,17 +537,17 @@ func TestGetStorageSummaryCapacityTrend_MockAvoidsPerPoolChartCacheFanout(t *tes previousEnabled := mock.IsMockEnabled() previousConfig := mock.GetConfig() t.Cleanup(func() { - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(previousConfig) if previousEnabled { - mock.SetEnabled(true) + mustSetMockEnabled(t, true) mock.SetMockConfig(previousConfig) } }) - mock.SetEnabled(false) + mustSetMockEnabled(t, false) mock.SetMockConfig(previousConfig) - mock.SetEnabled(true) + mustSetMockEnabled(t, true) monitor := newChartFallbackTestMonitor(t) points, oldestTimestamp := monitor.GetStorageSummaryCapacityTrend(24 * time.Hour) @@ -729,8 +729,8 @@ func TestSLO_GetGuestMetricsForChart_WithNativeHistoryFallback(t *testing.T) { func TestSLO_GetGuestMetricsForChartBatch_DoesNotStitchSparseStoreTailOntoCoveredInMemorySeries(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(previous) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Second) @@ -772,8 +772,8 @@ func TestSLO_GetGuestMetricsForChartBatch_DoesNotStitchSparseStoreTailOntoCovere func TestMockChartCacheInvalidatesAfterMockHistoryRefresh(t *testing.T) { previous := mock.IsMockEnabled() - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(previous) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, previous) }) monitor := newChartFallbackTestMonitor(t) now := time.Now().UTC().Truncate(time.Second) diff --git a/internal/monitoring/monitor_polling_test.go b/internal/monitoring/monitor_polling_test.go index 46e27d464..b6fbc7e65 100644 --- a/internal/monitoring/monitor_polling_test.go +++ b/internal/monitoring/monitor_polling_test.go @@ -600,9 +600,9 @@ func TestSyncUnifiedAppContainerMetricsSkipsMockOwnedTrueNASHistoryWhenMockEnabl }) previousMock := mock.IsMockEnabled() - mock.SetEnabled(true) + mustSetMockEnabled(t, true) t.Cleanup(func() { - mock.SetEnabled(previousMock) + mustSetMockEnabled(t, previousMock) }) cfg := metrics.DefaultConfig(t.TempDir()) @@ -696,9 +696,9 @@ func TestSyncUnifiedAgentMetricsSkipsMockOwnedProviderHistoryWhenMockEnabled(t * }) previousMock := mock.IsMockEnabled() - mock.SetEnabled(true) + mustSetMockEnabled(t, true) t.Cleanup(func() { - mock.SetEnabled(previousMock) + mustSetMockEnabled(t, previousMock) }) cfg := metrics.DefaultConfig(t.TempDir()) @@ -957,9 +957,9 @@ func TestSyncUnifiedStorageAndDiskMetricsSkipMockOwnedTrueNASHistoryWhenMockEnab }) previousMock := mock.IsMockEnabled() - mock.SetEnabled(true) + mustSetMockEnabled(t, true) t.Cleanup(func() { - mock.SetEnabled(previousMock) + mustSetMockEnabled(t, previousMock) }) cfg := metrics.DefaultConfig(t.TempDir()) diff --git a/internal/monitoring/monitor_unified_state_test.go b/internal/monitoring/monitor_unified_state_test.go index c9a162133..1764830a8 100644 --- a/internal/monitoring/monitor_unified_state_test.go +++ b/internal/monitoring/monitor_unified_state_test.go @@ -430,8 +430,8 @@ func TestMonitorUnifiedResourceSnapshotPrefersStoreFreshness(t *testing.T) { } func TestMonitorGetUnifiedReadStateOrSnapshotUsesCanonicalMockUnifiedResources(t *testing.T) { - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, false) }) graph := mock.CurrentFixtureGraph() legacyName := "" @@ -487,8 +487,8 @@ func TestMonitorGetUnifiedReadStateOrSnapshotUsesCanonicalMockUnifiedResources(t } func TestMonitorBuildBroadcastFrontendStateUsesCanonicalMockUnifiedResources(t *testing.T) { - mock.SetEnabled(true) - t.Cleanup(func() { mock.SetEnabled(false) }) + mustSetMockEnabled(t, true) + t.Cleanup(func() { mustSetMockEnabled(t, false) }) graph := mock.CurrentFixtureGraph() legacyName := "" diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go index a4ba5661d..a37ed8fd2 100644 --- a/internal/telemetry/telemetry_test.go +++ b/internal/telemetry/telemetry_test.go @@ -55,7 +55,9 @@ func TestGetOrCreateInstallID_ReusesExisting(t *testing.T) { func TestGetOrCreateInstallID_RegeneratesInvalid(t *testing.T) { dir := t.TempDir() // Write garbage. - os.WriteFile(filepath.Join(dir, installIDFile), []byte("not-a-uuid\n"), 0600) + if err := os.WriteFile(filepath.Join(dir, installIDFile), []byte("not-a-uuid\n"), 0600); err != nil { + t.Fatalf("write install id file: %v", err) + } id := getOrCreateInstallIDAt(dir, time.Date(2026, 3, 28, 12, 0, 0, 0, time.UTC)) if id == "" || id == "not-a-uuid" { @@ -283,7 +285,7 @@ func TestSend_Success(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { body, _ := io.ReadAll(r.Body) - json.Unmarshal(body, &lastPing) + _ = json.Unmarshal(body, &lastPing) received.Add(1) w.WriteHeader(http.StatusNoContent) })) diff --git a/internal/utils/gzip_test.go b/internal/utils/gzip_test.go index 5972cbf6e..2bb4afb05 100644 --- a/internal/utils/gzip_test.go +++ b/internal/utils/gzip_test.go @@ -122,7 +122,7 @@ func TestDecompressBodyIfGzipped_BombProtection(t *testing.T) { gz, _ := gzip.NewWriterLevel(&buf, gzip.BestCompression) // Write 2KB of zeros (compresses very well) payload := make([]byte, 2048) - gz.Write(payload) + _, _ = gz.Write(payload) gz.Close() req, _ := http.NewRequest("POST", "/", bytes.NewReader(buf.Bytes())) diff --git a/pkg/auth/policy_evaluator_test.go b/pkg/auth/policy_evaluator_test.go index 65eec4c84..1943c4ab1 100644 --- a/pkg/auth/policy_evaluator_test.go +++ b/pkg/auth/policy_evaluator_test.go @@ -26,7 +26,9 @@ func TestPolicyEvaluator(t *testing.T) { t.Run("Allow permission works", func(t *testing.T) { ctx := WithUser(context.Background(), "allow-user") - m.UpdateUserRoles("allow-user", []string{"allow-role"}) + if err := m.UpdateUserRoles("allow-user", []string{"allow-role"}); err != nil { + t.Fatalf("UpdateUserRoles: %v", err) + } allowed, err := evaluator.Authorize(ctx, "read", "nodes") if err != nil { @@ -51,7 +53,9 @@ func TestPolicyEvaluator(t *testing.T) { t.Run("Deny takes precedence over allow", func(t *testing.T) { ctx := WithUser(context.Background(), "deny-user") - m.UpdateUserRoles("deny-user", []string{"deny-role"}) + if err := m.UpdateUserRoles("deny-user", []string{"deny-role"}); err != nil { + t.Fatalf("UpdateUserRoles: %v", err) + } // deny-role has allow on nodes:* but deny on nodes:production allowed, err := evaluator.Authorize(ctx, "write", "nodes:test") @@ -73,7 +77,9 @@ func TestPolicyEvaluator(t *testing.T) { t.Run("Multiple roles combined", func(t *testing.T) { ctx := WithUser(context.Background(), "multi-user") - m.UpdateUserRoles("multi-user", []string{"allow-role", "extra-role"}) + if err := m.UpdateUserRoles("multi-user", []string{"allow-role", "extra-role"}); err != nil { + t.Fatalf("UpdateUserRoles: %v", err) + } // allow-role grants read:nodes, extra-role grants write:alerts allowed, err := evaluator.Authorize(ctx, "read", "nodes") @@ -153,8 +159,12 @@ func TestPolicyEvaluatorWithAttributes(t *testing.T) { }, }, } - m.SaveRole(condRole) - m.UpdateUserRoles("cond-user", []string{"cond-role"}) + if err := m.SaveRole(condRole); err != nil { + t.Fatalf("SaveRole: %v", err) + } + if err := m.UpdateUserRoles("cond-user", []string{"cond-role"}); err != nil { + t.Fatalf("UpdateUserRoles: %v", err) + } t.Run("Condition matches", func(t *testing.T) { ctx := WithUser(context.Background(), "cond-user") @@ -237,7 +247,9 @@ func TestPolicyEvaluatorWithInheritance(t *testing.T) { Name: "Parent", Permissions: []Permission{{Action: "read", Resource: "base"}}, } - m.SaveRole(parentRole) + if err := m.SaveRole(parentRole); err != nil { + t.Fatalf("SaveRole: %v", err) + } // Create child role childRole := Role{ @@ -246,9 +258,13 @@ func TestPolicyEvaluatorWithInheritance(t *testing.T) { ParentID: "parent", Permissions: []Permission{{Action: "write", Resource: "child"}}, } - m.SaveRole(childRole) + if err := m.SaveRole(childRole); err != nil { + t.Fatalf("SaveRole: %v", err) + } - m.UpdateUserRoles("inherit-user", []string{"child"}) + if err := m.UpdateUserRoles("inherit-user", []string{"child"}); err != nil { + t.Fatalf("UpdateUserRoles: %v", err) + } t.Run("Inherited permission works", func(t *testing.T) { ctx := WithUser(context.Background(), "inherit-user") @@ -293,7 +309,9 @@ func TestRBACAuthorizer(t *testing.T) { // Setup setupTestRoles(t, m) - m.UpdateUserRoles("normal-user", []string{"allow-role"}) + if err := m.UpdateUserRoles("normal-user", []string{"allow-role"}); err != nil { + t.Fatalf("UpdateUserRoles: %v", err) + } t.Run("Normal user authorization", func(t *testing.T) { ctx := WithUser(context.Background(), "normal-user") diff --git a/pkg/auth/sqlite_manager.go b/pkg/auth/sqlite_manager.go index 70cc6db13..58c8af78c 100644 --- a/pkg/auth/sqlite_manager.go +++ b/pkg/auth/sqlite_manager.go @@ -311,7 +311,9 @@ func (m *SQLiteManager) loadRolePermissions(roleID string) []Permission { } if conditions.Valid && conditions.String != "" { - json.Unmarshal([]byte(conditions.String), &perm.Conditions) + if err := json.Unmarshal([]byte(conditions.String), &perm.Conditions); err != nil { + log.Error().Err(err).Msg("Failed to parse permission conditions") + } } perms = append(perms, perm) @@ -396,7 +398,7 @@ func (m *SQLiteManager) SaveRoleWithContext(role Role, username string) error { if err != nil { return err } - defer tx.Rollback() + defer func() { _ = tx.Rollback() }() // Upsert role _, err = tx.Exec(` @@ -644,7 +646,7 @@ func (m *SQLiteManager) UpdateUserRolesWithContext(username string, roleIDs []st if err != nil { return err } - defer tx.Rollback() + defer func() { _ = tx.Rollback() }() // Delete existing assignments _, err = tx.Exec("DELETE FROM rbac_user_assignments WHERE username = ?", username) @@ -912,7 +914,9 @@ func (m *SQLiteManager) migrateFromFiles(dataDir string) error { // Check if migration is needed var roleCount int - m.db.QueryRow("SELECT COUNT(*) FROM rbac_roles WHERE is_built_in = 0").Scan(&roleCount) + if err := m.db.QueryRow("SELECT COUNT(*) FROM rbac_roles WHERE is_built_in = 0").Scan(&roleCount); err != nil { + log.Warn().Err(err).Msg("Failed to count custom roles before migration") + } if roleCount > 0 { return nil // Already have custom roles, skip migration } @@ -931,7 +935,9 @@ func (m *SQLiteManager) migrateFromFiles(dataDir string) error { log.Info().Int("count", len(roles)).Msg("Migrated roles from file") // Rename old file - os.Rename(rolesFile, rolesFile+".bak") + if err := os.Rename(rolesFile, rolesFile+".bak"); err != nil { + log.Warn().Err(err).Msg("Failed to rename migrated roles file") + } } } @@ -947,7 +953,9 @@ func (m *SQLiteManager) migrateFromFiles(dataDir string) error { log.Info().Int("count", len(assignments)).Msg("Migrated assignments from file") // Rename old file - os.Rename(assignmentsFile, assignmentsFile+".bak") + if err := os.Rename(assignmentsFile, assignmentsFile+".bak"); err != nil { + log.Warn().Err(err).Msg("Failed to rename migrated assignments file") + } } } diff --git a/pkg/auth/sqlite_manager_bench_test.go b/pkg/auth/sqlite_manager_bench_test.go index 7f890dde1..28ef50b2e 100644 --- a/pkg/auth/sqlite_manager_bench_test.go +++ b/pkg/auth/sqlite_manager_bench_test.go @@ -177,7 +177,7 @@ func BenchmarkAuthorize(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { req := requests[i%len(requests)] - authorizer.Authorize(req.ctx, req.action, req.resource) + _, _ = authorizer.Authorize(req.ctx, req.action, req.resource) } } diff --git a/pkg/auth/sqlite_manager_test.go b/pkg/auth/sqlite_manager_test.go index ebcb161d4..0655cd3e7 100644 --- a/pkg/auth/sqlite_manager_test.go +++ b/pkg/auth/sqlite_manager_test.go @@ -333,15 +333,21 @@ func TestSQLiteManagerCircularInheritance(t *testing.T) { // Create role A roleA := Role{ID: "role-a", Name: "Role A", Permissions: []Permission{{Action: "read", Resource: "a"}}} - m.SaveRole(roleA) + if err := m.SaveRole(roleA); err != nil { + t.Fatalf("SaveRole: %v", err) + } // Create role B with parent A roleB := Role{ID: "role-b", Name: "Role B", ParentID: "role-a", Permissions: []Permission{{Action: "read", Resource: "b"}}} - m.SaveRole(roleB) + if err := m.SaveRole(roleB); err != nil { + t.Fatalf("SaveRole: %v", err) + } // Create role C with parent B roleC := Role{ID: "role-c", Name: "Role C", ParentID: "role-b", Permissions: []Permission{{Action: "read", Resource: "c"}}} - m.SaveRole(roleC) + if err := m.SaveRole(roleC); err != nil { + t.Fatalf("SaveRole: %v", err) + } // Try to make A inherit from C (creating cycle) roleA.ParentID = "role-c" @@ -435,8 +441,12 @@ func TestSQLiteManagerPersistence(t *testing.T) { ParentID: RoleViewer, Permissions: []Permission{{Action: "write", Resource: "persist", Effect: EffectAllow}}, } - m1.SaveRole(role) - m1.AssignRole("persist-user", "persist-role") + if err := m1.SaveRole(role); err != nil { + t.Fatalf("SaveRole: %v", err) + } + if err := m1.AssignRole("persist-user", "persist-role"); err != nil { + t.Fatalf("AssignRole: %v", err) + } m1.Close() // Reopen and verify @@ -544,7 +554,9 @@ func TestSQLiteManagerChangeLogRetention(t *testing.T) { Name: "Retention Test " + string(rune('a'+i)), Permissions: []Permission{{Action: "read", Resource: "test"}}, } - m.SaveRole(role) + if err := m.SaveRole(role); err != nil { + t.Fatalf("SaveRole: %v", err) + } // Add slight delay to ensure different timestamps time.Sleep(10 * time.Millisecond) } diff --git a/pkg/metrics/store.go b/pkg/metrics/store.go index 8ac407e47..2e03a8f2d 100644 --- a/pkg/metrics/store.go +++ b/pkg/metrics/store.go @@ -332,7 +332,7 @@ func (s *Store) ensureMetricsUniqueIndex() error { if txErr != nil { return fmt.Errorf("begin dedupe transaction: %w", txErr) } - defer tx.Rollback() + defer func() { _ = tx.Rollback() }() _, txErr = tx.Exec(` DELETE FROM metrics @@ -396,7 +396,7 @@ func (s *Store) migrateLegacyHostResourceType() { log.Warn().Err(err).Msg("Failed to start legacy host->agent metrics migration") return } - defer tx.Rollback() + defer func() { _ = tx.Rollback() }() // Reinsert legacy rows with canonical type, keeping any existing canonical // records when duplicates collide with the unique index. @@ -780,7 +780,7 @@ func (s *Store) writeBatch(metrics []bufferedMetric) { max_value = excluded.max_value `) if err != nil { - tx.Rollback() + _ = tx.Rollback() log.Error().Err(err). Str("component", "metrics_store"). Str("action", "prepare_write_stmt"). @@ -1583,7 +1583,7 @@ func (s *Store) rollupTier(fromTier, toTier Tier, bucketSize, minAge time.Durati log.Error().Err(err).Str("tier", string(fromTier)).Msg("Failed to begin rollup transaction") return } - defer tx.Rollback() + defer func() { _ = tx.Rollback() }() _, err = tx.Exec(` INSERT OR IGNORE INTO metrics (resource_type, resource_id, metric_type, value, min_value, max_value, timestamp, tier) @@ -1630,7 +1630,7 @@ func (s *Store) rollupCandidate(resourceType, resourceID, metricType string, fro if err != nil { return } - defer itx.Rollback() + defer func() { _ = itx.Rollback() }() // Aggregate data into buckets _, err = itx.Exec(` @@ -1659,7 +1659,13 @@ func (s *Store) rollupCandidate(resourceType, resourceID, metricType string, fro return } - itx.Commit() + if err := itx.Commit(); err != nil { + log.Warn().Err(err). + Str("resource", resourceID). + Str("from", string(fromTier)). + Str("to", string(toTier)). + Msg("Failed to commit rollup transaction") + } } func (s *Store) getMetaInt(key string) (int64, bool) { diff --git a/pkg/server/server.go b/pkg/server/server.go index 6802b6bb4..7c5a4d5f2 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -433,7 +433,11 @@ func Run(ctx context.Context, version string) error { reaper.OnBeforeDelete = func(orgID string) error { return router.CleanupTenant(ctx, orgID) } - go reaper.Run(ctx) + go func() { + if err := reaper.Run(ctx); err != nil { + log.Error().Err(err).Msg("Hosted tenant reaper exited with error") + } + }() log.Info().Msg("Hosted tenant reaper started") } @@ -803,7 +807,9 @@ shutdown: // Ensure mock-mode background update ticker is stopped before process exit. if mock.IsMockEnabled() { - mock.SetEnabled(false) + if err := mock.SetEnabled(false); err != nil { + log.Warn().Err(err).Msg("Failed to disable mock mode during shutdown") + } } cancel() diff --git a/scripts/installtests/install_sh_test.go b/scripts/installtests/install_sh_test.go index 22d3b4fc1..0f2ab7fdf 100644 --- a/scripts/installtests/install_sh_test.go +++ b/scripts/installtests/install_sh_test.go @@ -1084,7 +1084,7 @@ func TestAPIDeregistrationCurl(t *testing.T) { gotPath = r.URL.Path gotHeaders = r.Header.Clone() body, _ := io.ReadAll(r.Body) - json.Unmarshal(body, &gotBody) + _ = json.Unmarshal(body, &gotBody) w.WriteHeader(200) w.Write([]byte(`{"success":true}`)) })) @@ -1143,7 +1143,7 @@ func TestAPIDeregistrationCurlWithoutToken(t *testing.T) { gotPath = r.URL.Path gotHeaders = r.Header.Clone() body, _ := io.ReadAll(r.Body) - json.Unmarshal(body, &gotBody) + _ = json.Unmarshal(body, &gotBody) w.WriteHeader(200) w.Write([]byte(`{"success":true}`)) })) diff --git a/tests/migration/v5_full_upgrade_test.go b/tests/migration/v5_full_upgrade_test.go index f369104fd..6657b8039 100644 --- a/tests/migration/v5_full_upgrade_test.go +++ b/tests/migration/v5_full_upgrade_test.go @@ -170,7 +170,7 @@ func newMigrationTestServer(t *testing.T, dataDir string) *httptest.Server { t.Helper() t.Setenv("PULSE_MOCK_MODE", "true") - mock.SetEnabled(true) + require.NoError(t, mock.SetEnabled(true), "enable mock mode") cfg := &config.Config{ ConfigPath: dataDir, @@ -192,7 +192,7 @@ func newMigrationTestServer(t *testing.T, dataDir string) *httptest.Server { var err error monitor, err = monitoring.New(cfg) require.NoError(t, err, "monitor should initialize against v5 data dir") - monitor.SetMockMode(true) + require.NoError(t, monitor.SetMockMode(true), "set monitor mock mode") hub.SetStateGetter(func(orgID string) interface{} { return monitor.GetState().ToFrontend() @@ -218,7 +218,9 @@ func newMigrationTestServer(t *testing.T, dataDir string) *httptest.Server { monitor.StopDiscoveryService() monitor.Stop() hub.Stop() - mock.SetEnabled(false) + if err := mock.SetEnabled(false); err != nil { + t.Errorf("disable mock mode: %v", err) + } }) return srv