From fb9584aae68fe46dbe19e2d96620a201ee62d116 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 16 Jul 2026 15:17:45 +0100 Subject: [PATCH] Add Go branch-coverage tests for alerts monitoring model-resolution ai and licensing helpers Adds table-driven branch-coverage unit tests for previously uncovered pure functions across internal/alerts/config, internal/ai/modelresolution, internal/monitoring, internal/ai, internal/ai/qualification and pkg/licensing. New test files only, with no source changes. Covers per-subsystem alert-default normalization, configured model and provider resolution, fleet-doctor identity and cluster-endpoint helpers, docker-state and infrastructure-key mapping, investigation and SMART issue helpers, JSON and autonomy normalization, and licensing feature-tier resolution. 65 TestBranchCov functions in 7 files, all vet and gofmt clean. --- ...ai_core_pure_helpers_branchcov0716_test.go | 309 +++++ ...elresolution_resolve_branchcov0716_test.go | 565 +++++++++ ...i_qualification_pure_branchcov0716_test.go | 180 +++ ...g_normalize_defaults_branchcov0716_test.go | 1072 +++++++++++++++++ ...fleet_doctor_helpers_branchcov0716_test.go | 318 +++++ ...onitoring_infra_keys_branchcov0716_test.go | 294 +++++ ...ensing_features_pure_branchcov0716_test.go | 364 ++++++ 7 files changed, 3102 insertions(+) create mode 100644 internal/ai/ai_core_pure_helpers_branchcov0716_test.go create mode 100644 internal/ai/modelresolution/modelresolution_resolve_branchcov0716_test.go create mode 100644 internal/ai/qualification/ai_qualification_pure_branchcov0716_test.go create mode 100644 internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go create mode 100644 internal/monitoring/monitoring_fleet_doctor_helpers_branchcov0716_test.go create mode 100644 internal/monitoring/monitoring_infra_keys_branchcov0716_test.go create mode 100644 pkg/licensing/licensing_features_pure_branchcov0716_test.go diff --git a/internal/ai/ai_core_pure_helpers_branchcov0716_test.go b/internal/ai/ai_core_pure_helpers_branchcov0716_test.go new file mode 100644 index 000000000..dcff54eb5 --- /dev/null +++ b/internal/ai/ai_core_pure_helpers_branchcov0716_test.go @@ -0,0 +1,309 @@ +package ai + +import ( + "strconv" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/pkg/aicontracts" +) + +// TestBranchCovFindingGetSetLoopState pins the real behaviour of the +// Finding.GetLoopState / Finding.SetLoopState pair (findings.go). SetLoopState +// is a plain field assignment with no validation and no derivation side-effect +// (it deliberately does NOT call syncLoopState/deriveLoopState), so the test +// asserts: the zero value, exact round-trip, overwrite semantics, explicit +// clearing, and that arbitrary/unknown values pass through unnormalised. +func TestBranchCovFindingGetSetLoopState(t *testing.T) { + // Zero-value Finding: GetLoopState returns the empty default. + zero := &Finding{} + if got := zero.GetLoopState(); got != "" { + t.Fatalf("zero-value GetLoopState = %q, want empty", got) + } + + // SetLoopState mutates the underlying field; GetLoopState observes the + // exact value with no transformation. + f := &Finding{} + f.SetLoopState(string(FindingLoopStateInvestigating)) + if got := f.GetLoopState(); got != string(FindingLoopStateInvestigating) { + t.Fatalf("after SetLoopState(investigating): GetLoopState = %q, want %q", + got, FindingLoopStateInvestigating) + } + + // SetLoopState overwrites a prior value (no append/merge). + f.SetLoopState(string(FindingLoopStateResolved)) + if got := f.GetLoopState(); got != string(FindingLoopStateResolved) { + t.Fatalf("after overwrite: GetLoopState = %q, want %q", got, FindingLoopStateResolved) + } + + // SetLoopState("") explicitly clears the stored value. + f.SetLoopState("") + if got := f.GetLoopState(); got != "" { + t.Fatalf("after SetLoopState(\"\"): GetLoopState = %q, want empty", got) + } + + // Arbitrary/unknown string round-trips verbatim (no validation, no + // re-derivation through deriveLoopState). + const arbitrary = " not-a-known-state " + f.SetLoopState(arbitrary) + if got := f.GetLoopState(); got != arbitrary { + t.Fatalf("arbitrary round-trip: GetLoopState = %q, want %q", got, arbitrary) + } + + // The mutation is observable on the same struct via the exported field too, + // confirming Get/Set operate on the single LoopState field. + if f.LoopState != arbitrary { + t.Fatalf("struct field LoopState = %q, want %q", f.LoopState, arbitrary) + } +} + +// TestBranchCovInvestigationRecordStatusIsTerminal exercises every arm of the +// switch in investigationRecordStatusIsTerminal (investigation_records.go): +// the three grouped terminal cases and the default fall-through, including the +// zero-value status and a status outside the known enum set. +func TestBranchCovInvestigationRecordStatusIsTerminal(t *testing.T) { + cases := []struct { + name string + status aicontracts.InvestigationStatus + want bool + }{ + // Explicit terminal arms. + {"completed_is_terminal", aicontracts.InvestigationStatusCompleted, true}, + {"failed_is_terminal", aicontracts.InvestigationStatusFailed, true}, + {"needs_attention_is_terminal", aicontracts.InvestigationStatusNeedsAttention, true}, + // Explicit non-terminal statuses -> default arm. + {"pending_not_terminal", aicontracts.InvestigationStatusPending, false}, + {"running_not_terminal", aicontracts.InvestigationStatusRunning, false}, + // Default arm: zero-value status. + {"empty_status_not_terminal", aicontracts.InvestigationStatus(""), false}, + // Default arm: status outside the known enum set. + {"unknown_status_not_terminal", aicontracts.InvestigationStatus("frobnicated"), false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := investigationRecordStatusIsTerminal(tc.status) + if got != tc.want { + t.Fatalf("investigationRecordStatusIsTerminal(%q) = %v, want %v", + tc.status, got, tc.want) + } + }) + } +} + +// TestBranchCovExtractIncidentResourceIdentifier exercises every reachable +// branch of extractIncidentResourceIdentifier (service.go): the trimmed +// targetID early-return (and its precedence over ctx), the whitespace-only / +// empty targetID fall-through, the nil-ctx guard, each of the three context +// keys in iteration order, type-assertion failure on a non-string value, the +// non-empty-after-trim guard, and the empty-result paths. +func TestBranchCovExtractIncidentResourceIdentifier(t *testing.T) { + cases := []struct { + name string + targetID string + ctx map[string]interface{} + want string + }{ + { + // Branch: targetID non-empty after trim -> returned trimmed; ctx ignored. + name: "target_id_present_ignores_ctx", + targetID: " node/pve-1/qemu/100 ", + ctx: map[string]interface{}{"resourceID": "should-be-ignored"}, + want: "node/pve-1/qemu/100", + }, + { + // Branch: targetID whitespace-only -> trimmed to "", falls through to ctx. + name: "whitespace_only_target_id_falls_through_to_ctx", + targetID: " \t\n ", + ctx: map[string]interface{}{"resourceID": "from-ctx"}, + want: "from-ctx", + }, + { + // Branch: empty targetID + nil ctx -> "". + name: "empty_target_id_nil_ctx_returns_empty", + targetID: "", + ctx: nil, + want: "", + }, + { + // Branch: ctx["resourceID"] hit, value trimmed on output. + name: "resourceID_key_value_trimmed", + targetID: "", + ctx: map[string]interface{}{"resourceID": " vm-42 "}, + want: "vm-42", + }, + { + // Branch: ctx["resourceID"] present but whitespace-only -> skipped, + // falls through to next key in iteration order. + name: "resourceID_whitespace_only_skipped_falls_to_resource_id", + targetID: "", + ctx: map[string]interface{}{ + "resourceID": " ", + "resource_id": "snake-case-wins", + }, + want: "snake-case-wins", + }, + { + // Branch: ctx["resourceID"] is a non-string type -> type assertion + // fails, skipped, falls through to "resource_id". + name: "resourceID_non_string_skipped_falls_to_resource_id", + targetID: "", + ctx: map[string]interface{}{ + "resourceID": 12345, + "resource_id": "int-coerced-fallthrough", + }, + want: "int-coerced-fallthrough", + }, + { + // Branch: only the snake_case key present. + name: "resource_id_key_hit", + targetID: "", + ctx: map[string]interface{}{"resource_id": " storage/pool-0 "}, + want: "storage/pool-0", + }, + { + // Branch: only the camelCase resourceId key present (last in order). + name: "resourceId_key_hit", + targetID: "", + ctx: map[string]interface{}{"resourceId": "ct-7"}, + want: "ct-7", + }, + { + // Branch: precedence — resourceID wins over resource_id and resourceId. + name: "resourceID_precedence_over_resource_id_and_resourceId", + targetID: "", + ctx: map[string]interface{}{ + "resourceID": "first", + "resource_id": "second", + "resourceId": "third", + }, + want: "first", + }, + { + // Branch: resource_id wins over resourceId. + name: "resource_id_precedence_over_resourceId", + targetID: "", + ctx: map[string]interface{}{ + "resource_id": "second", + "resourceId": "third", + }, + want: "second", + }, + { + // Branch: none of the known keys present -> "". + name: "no_known_keys_returns_empty", + targetID: "", + ctx: map[string]interface{}{"unrelated": "x", "count": 3}, + want: "", + }, + { + // Branch: all known keys present but empty/whitespace -> "". + name: "all_known_keys_empty_returns_empty", + targetID: "", + ctx: map[string]interface{}{ + "resourceID": "", + "resource_id": " ", + "resourceId": "", + }, + want: "", + }, + { + // Branch: empty targetID + empty (non-nil) ctx -> "". + name: "empty_target_id_empty_ctx_returns_empty", + targetID: "", + ctx: map[string]interface{}{}, + want: "", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := extractIncidentResourceIdentifier(tc.targetID, tc.ctx) + if got != tc.want { + t.Fatalf("extractIncidentResourceIdentifier(%q, ctx) = %q, want %q", + tc.targetID, got, tc.want) + } + }) + } +} + +// TestBranchCovAppendSMARTInt64Issue exercises every reachable branch of +// appendSMARTInt64Issue (patrol_ai.go) and the value filter it delegates to +// (appendSMARTInt64ValueIssue): the nil-pointer early return, the delegated +// value<=0 drop for zero and negative, the positive-value append with exact +// "label=%d" formatting, accumulation/ordering, and appending into a nil slice. +func TestBranchCovAppendSMARTInt64Issue(t *testing.T) { + int64ptr := func(v int64) *int64 { return &v } + + // Branch: value == nil -> early return, parts unchanged. + t.Run("nil_value_is_noop", func(t *testing.T) { + parts := []string{"existing"} + appendSMARTInt64Issue(&parts, "reallocated sectors", nil) + if len(parts) != 1 || parts[0] != "existing" { + t.Fatalf("nil value mutated parts; got %v", parts) + } + }) + + // Branch: value points to 0 -> delegated filter drops it. + t.Run("zero_value_is_noop", func(t *testing.T) { + parts := []string{} + appendSMARTInt64Issue(&parts, "pending sectors", int64ptr(0)) + if len(parts) != 0 { + t.Fatalf("zero value should not append; got %v", parts) + } + }) + + // Branch: value points to negative -> delegated filter drops it. + t.Run("negative_value_is_noop", func(t *testing.T) { + parts := []string{} + appendSMARTInt64Issue(&parts, "offline uncorrectable", int64ptr(-5)) + if len(parts) != 0 { + t.Fatalf("negative value should not append; got %v", parts) + } + }) + + // Branch: value points to positive -> appends "label=%d". + t.Run("positive_value_appends", func(t *testing.T) { + parts := []string{} + appendSMARTInt64Issue(&parts, "UDMA CRC errors", int64ptr(7)) + if len(parts) != 1 || parts[0] != "UDMA CRC errors=7" { + t.Fatalf("positive append: got %v", parts) + } + }) + + // Branch: repeated calls accumulate in call order, with nil/zero skipped. + t.Run("accumulates_in_order_skipping_nil_and_zero", func(t *testing.T) { + parts := []string{} + appendSMARTInt64Issue(&parts, "reallocated sectors", int64ptr(2)) + appendSMARTInt64Issue(&parts, "media errors", int64ptr(9)) + appendSMARTInt64Issue(&parts, "pending sectors", nil) // skipped (nil) + appendSMARTInt64Issue(&parts, "offline uncorrectable", int64ptr(0)) // skipped (<=0) + want := []string{"reallocated sectors=2", "media errors=9"} + if len(parts) != len(want) { + t.Fatalf("accumulate length: got %v want %v", parts, want) + } + for i := range want { + if parts[i] != want[i] { + t.Fatalf("accumulate[%d]: got %q want %q (full: %v)", i, parts[i], want[i], parts) + } + } + }) + + // Branch: appending into a nil []string (parts points to nil) exercises + // append's grow-from-nil path through the helper. + t.Run("appends_into_nil_slice", func(t *testing.T) { + var parts []string + appendSMARTInt64Issue(&parts, "media errors", int64ptr(3)) + if len(parts) != 1 || parts[0] != "media errors=3" { + t.Fatalf("nil-slice append: got %v", parts) + } + }) + + // Formatting: a large positive int64 renders with %d (no overflow/truncation). + t.Run("large_value_formatted_as_decimal", func(t *testing.T) { + parts := []string{} + const big int64 = 1<<62 + 1 + appendSMARTInt64Issue(&parts, "media errors", int64ptr(big)) + want := "media errors=" + strconv.FormatInt(big, 10) + if len(parts) != 1 || parts[0] != want { + t.Fatalf("large value formatting: got %v want [%q]", parts, want) + } + }) +} diff --git a/internal/ai/modelresolution/modelresolution_resolve_branchcov0716_test.go b/internal/ai/modelresolution/modelresolution_resolve_branchcov0716_test.go new file mode 100644 index 000000000..67f85d229 --- /dev/null +++ b/internal/ai/modelresolution/modelresolution_resolve_branchcov0716_test.go @@ -0,0 +1,565 @@ +package modelresolution + +import ( + "context" + "strings" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +// These branch-coverage tests target the model-resolution entry points and the +// selectedModelProviderError helper. They deliberately avoid the live +// provider-catalog paths (providers.NewForProvider + ListModels), which have no +// test seam in this package and would require real network access; every +// deterministic input-validation, preferred-model short-circuit, delegation, +// and error-propagation branch is exercised instead. See GLM_REPORT.md for the +// small set of branches that are not sensibly coverable in isolation. + +// TestBranchCovResolveConfiguredModel covers every deterministic branch of +// ResolveConfiguredModel: nil config, explicit usable model (including the +// heuristic no-prefix resolution and the non-chat path that accepts +// specialized models), explicit model on an unconfigured provider (error +// propagated from selectedModelProvider), no configured providers, and +// delegation to ResolveConfiguredProviderModel where a provider-scoped +// preferred model wins without touching the network. +func TestBranchCovResolveConfiguredModel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg *config.AIConfig + want string + wantErr string // non-empty substring expected in the error + }{ + { + name: "nil config returns config-is-nil error", + cfg: nil, + wantErr: "config is nil", + }, + { + name: "explicit usable model on configured provider is returned verbatim", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "openai:gpt-4o", + }, + want: "openai:gpt-4o", + }, + { + name: "explicit model with no provider prefix resolved via heuristic to configured openai", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "gpt-4o", + }, + want: "gpt-4o", + }, + { + name: "explicit specialized model on configured provider is usable on the non-chat path", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "openai:text-embedding-3", + }, + want: "openai:text-embedding-3", + }, + { + name: "explicit model on unconfigured provider surfaces provider-not-configured error", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "deepseek:deepseek-v4", + }, + wantErr: "deepseek provider is not configured", + }, + { + name: "explicit model defaulting to unconfigured ollama via heuristic surfaces ollama error", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "totally-unknown-local-model", + }, + wantErr: "ollama provider is not configured", + }, + { + name: "no explicit model and no configured providers returns no-provider-configured error", + cfg: &config.AIConfig{}, + wantErr: "no provider configured", + }, + { + name: "no explicit model delegates to first configured provider and resolves preferred without network", + cfg: &config.AIConfig{ + // Only Ollama is configured, so configuredProviders[0] == "ollama". + // Model is empty so ResolveConfiguredModel delegates; the + // provider-scoped ChatModel is picked as Ollama's preferred model + // and short-circuits before any catalog call. + OllamaBaseURL: "http://localhost:11434", + ChatModel: "ollama:qwen3:8b", + }, + want: "ollama:qwen3:8b", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := ResolveConfiguredModel(context.Background(), tt.cfg) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("ResolveConfiguredModel() error = nil, want containing %q (got %q)", tt.wantErr, got) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("ResolveConfiguredModel() error = %q, want containing %q", err.Error(), tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("ResolveConfiguredModel() error = %v, want nil", err) + } + if got != tt.want { + t.Fatalf("ResolveConfiguredModel() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestBranchCovResolveConfiguredChatModel covers every deterministic branch of +// ResolveConfiguredChatModel: nil config, explicit chat-suitable model, +// explicit specialized model on a configured provider (which reaches the +// chat-variant "not usable for Assistant chat" fallthrough because the provider +// IS configured but the model is not chat-suitable), explicit model on an +// unconfigured provider, no configured providers, and delegation to +// ResolveConfiguredChatProviderModel via a provider-scoped PatrolModel. +func TestBranchCovResolveConfiguredChatModel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg *config.AIConfig + want string + wantErr string + }{ + { + name: "nil config returns config-is-nil error", + cfg: nil, + wantErr: "config is nil", + }, + { + name: "explicit chat-suitable model returned verbatim", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + ChatModel: "openai:gpt-4o", + }, + want: "openai:gpt-4o", + }, + { + name: "explicit specialized model on configured provider is rejected as not-usable-for-chat", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + ChatModel: "openai:text-embedding-3", + }, + wantErr: "not usable for Assistant chat", + }, + { + name: "explicit realtime model on configured provider rejected as not-usable-for-chat", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + ChatModel: "openai:gpt-4o-realtime", + }, + wantErr: "not usable for Assistant chat", + }, + { + name: "explicit model on unconfigured provider surfaces provider-not-configured", + cfg: &config.AIConfig{ + OpenRouterAPIKey: "sk-or-test", + ChatModel: "deepseek:deepseek-v4", + }, + wantErr: "deepseek provider is not configured", + }, + { + name: "no explicit model and no configured providers returns no-provider-configured error", + cfg: &config.AIConfig{}, + wantErr: "no provider configured", + }, + { + name: "no explicit model delegates and resolves provider-scoped preferred chat model", + cfg: &config.AIConfig{ + // Neither ChatModel nor Model is set, so GetChatModel() == "" and + // ResolveConfiguredChatModel delegates to the first configured + // provider. PatrolModel is provider-scoped to OpenAI and is + // chat-suitable, so it wins without a catalog call. + OpenAIAPIKey: "sk-test", + PatrolModel: "openai:gpt-4o", + }, + want: "openai:gpt-4o", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := ResolveConfiguredChatModel(context.Background(), tt.cfg) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("ResolveConfiguredChatModel() error = nil, want containing %q (got %q)", tt.wantErr, got) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("ResolveConfiguredChatModel() error = %q, want containing %q", err.Error(), tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("ResolveConfiguredChatModel() error = %v, want nil", err) + } + if got != tt.want { + t.Fatalf("ResolveConfiguredChatModel() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestBranchCovResolvePreferredModelForProvider covers: nil config, the +// preferred-model short-circuit win, and the fall-through-to-delegate path in +// several flavours (preferred present but on an unconfigured provider, and an +// empty preferred routing into the delegate's provider validation branches). +func TestBranchCovResolvePreferredModelForProvider(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg *config.AIConfig + provider string + want string + wantErr string + }{ + { + name: "nil config returns config-is-nil error", + cfg: nil, + provider: "openai", + wantErr: "config is nil", + }, + { + name: "provider-scoped preferred usable model wins", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "openai:gpt-4o", + }, + provider: "openai", + want: "openai:gpt-4o", + }, + { + name: "preferred present but provider unconfigured falls through to delegate provider-not-configured", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "deepseek:deepseek-v4", + }, + provider: "deepseek", + wantErr: "deepseek provider is not configured", + }, + { + name: "empty preferred and empty provider delegate to provider-required error", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "", + wantErr: "provider is required", + }, + { + name: "empty preferred and quickstart provider delegate to retired error", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "quickstart", + wantErr: "quickstart provider is retired", + }, + { + name: "empty preferred and unconfigured provider delegate to provider-not-configured", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "anthropic", + wantErr: "anthropic provider is not configured", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := ResolvePreferredModelForProvider(context.Background(), tt.cfg, tt.provider) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("ResolvePreferredModelForProvider() error = nil, want containing %q (got %q)", tt.wantErr, got) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("ResolvePreferredModelForProvider() error = %q, want containing %q", err.Error(), tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("ResolvePreferredModelForProvider() error = %v, want nil", err) + } + if got != tt.want { + t.Fatalf("ResolvePreferredModelForProvider() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestBranchCovResolveConfiguredProviderModel covers the deterministic branches +// of ResolveConfiguredProviderModel (chatOnly=false): nil config, empty and +// whitespace provider, retired quickstart, unconfigured provider, and the +// preferred-model short-circuit (including whitespace trimming of the provider +// argument and the non-chat acceptance of a specialized preferred model). +func TestBranchCovResolveConfiguredProviderModel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg *config.AIConfig + provider string + want string + wantErr string + }{ + { + name: "nil config returns config-is-nil error", + cfg: nil, + provider: "openai", + wantErr: "config is nil", + }, + { + name: "empty provider returns provider-required error", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "", + wantErr: "provider is required", + }, + { + name: "whitespace-only provider is trimmed to empty and returns provider-required", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: " ", + wantErr: "provider is required", + }, + { + name: "quickstart provider returns retired error", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "quickstart", + wantErr: "quickstart provider is retired", + }, + { + name: "unconfigured provider returns provider-not-configured", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "anthropic", + wantErr: "anthropic provider is not configured", + }, + { + name: "whitespace-padded configured provider trimmed and preferred model wins", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + PatrolModel: "openai:gpt-4o", + }, + provider: " openai ", + want: "openai:gpt-4o", + }, + { + name: "specialized preferred model wins on the non-chat path", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + Model: "openai:text-embedding-3", + }, + provider: "openai", + want: "openai:text-embedding-3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := ResolveConfiguredProviderModel(context.Background(), tt.cfg, tt.provider) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("ResolveConfiguredProviderModel() error = nil, want containing %q (got %q)", tt.wantErr, got) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("ResolveConfiguredProviderModel() error = %q, want containing %q", err.Error(), tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("ResolveConfiguredProviderModel() error = %v, want nil", err) + } + if got != tt.want { + t.Fatalf("ResolveConfiguredProviderModel() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestBranchCovResolveConfiguredChatProviderModel covers the deterministic +// branches of ResolveConfiguredChatProviderModel (chatOnly=true): nil config, +// empty/whitespace provider, retired quickstart, unconfigured provider, and the +// chat-suitable preferred-model short-circuit. +func TestBranchCovResolveConfiguredChatProviderModel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg *config.AIConfig + provider string + want string + wantErr string + }{ + { + name: "nil config returns config-is-nil error", + cfg: nil, + provider: "openai", + wantErr: "config is nil", + }, + { + name: "empty provider returns provider-required error", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "", + wantErr: "provider is required", + }, + { + name: "whitespace-only provider is trimmed to empty and returns provider-required", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "\t", + wantErr: "provider is required", + }, + { + name: "quickstart provider returns retired error", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "quickstart", + wantErr: "quickstart provider is retired", + }, + { + name: "unconfigured provider returns provider-not-configured", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + provider: "gemini", + wantErr: "gemini provider is not configured", + }, + { + name: "chat-suitable provider-scoped preferred model wins", + cfg: &config.AIConfig{ + OpenAIAPIKey: "sk-test", + PatrolModel: "openai:gpt-4o", + }, + provider: "openai", + want: "openai:gpt-4o", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := ResolveConfiguredChatProviderModel(context.Background(), tt.cfg, tt.provider) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("ResolveConfiguredChatProviderModel() error = nil, want containing %q (got %q)", tt.wantErr, got) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("ResolveConfiguredChatProviderModel() error = %q, want containing %q", err.Error(), tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("ResolveConfiguredChatProviderModel() error = %v, want nil", err) + } + if got != tt.want { + t.Fatalf("ResolveConfiguredChatProviderModel() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestBranchCovSelectedModelProviderError exercises selectedModelProviderError +// directly. It covers every selectedModelProvider propagation arm (empty model, +// quickstart provider, nil config, unconfigured provider reached via the +// default-Ollama heuristic) AND the fallthrough "not usable with the current +// Pulse Assistant config" arm. The fallthrough arm is only reachable by a +// direct call: when ResolveConfiguredModel invokes selectedModelProviderError +// it has already established IsModelUsableWithConfig==false, whose provider +// checks are equivalent to selectedModelProvider's, so selectedModelProvider +// always returns an error there and the fallthrough never runs in production +// (see GLM_REPORT.md). +func TestBranchCovSelectedModelProviderError(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg *config.AIConfig + model string + wantErr string + }{ + { + name: "empty model propagates selected-model-route-is-empty", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + model: "", + wantErr: "selected model route is empty", + }, + { + name: "whitespace model propagates selected-model-route-is-empty", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + model: " ", + wantErr: "selected model route is empty", + }, + { + name: "quickstart provider propagates retired error", + cfg: &config.AIConfig{}, + model: "quickstart:gpt-4o", + wantErr: "quickstart provider is retired", + }, + { + name: "nil config propagates provider-not-configured-for-route", + cfg: nil, + model: "openai:gpt-4o", + wantErr: "openai provider is not configured for selected model route", + }, + { + name: "default-ollama heuristic on configured-openai-only propagates ollama-not-configured-for-route", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + model: "totally-unknown-local-model", + wantErr: "ollama provider is not configured for selected model route", + }, + { + name: "configured provider fallthrough returns not-usable-with-current-config", + cfg: &config.AIConfig{OpenAIAPIKey: "sk-test"}, + model: "openai:gpt-4o", + wantErr: "not usable with the current Pulse Assistant config", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := selectedModelProviderError(tt.cfg, tt.model) + if err == nil { + t.Fatalf("selectedModelProviderError(%q) = nil, want error containing %q", tt.model, tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("selectedModelProviderError(%q) = %q, want containing %q", tt.model, err.Error(), tt.wantErr) + } + }) + } +} + +// TestBranchCovResolveConfiguredModel_NeverReachesNotUsableFallthrough pins the +// dead-code observation described above: for every explicit-model input that +// drives ResolveConfiguredModel into its error branch, the surfaced error is +// always selectedModelProvider's propagation, never the +// "not usable with the current Pulse Assistant config" fallthrough message. +func TestBranchCovResolveConfiguredModel_NeverReachesNotUsableFallthrough(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + cfg *config.AIConfig + model string + }{ + {name: "unconfigured provider", cfg: &config.AIConfig{OpenAIAPIKey: "sk"}, model: "deepseek:deepseek-v4"}, + {name: "default ollama heuristic unconfigured", cfg: &config.AIConfig{OpenAIAPIKey: "sk"}, model: "weird-local-name"}, + // A quickstart Model is intentionally omitted here: GetModel() runs it + // through NormalizeQuickstartModelString, which collapses it to "" so + // ResolveConfiguredModel treats it as "no explicit model" and never + // reaches selectedModelProviderError at all. + } + for _, tc := range cases { + tc.cfg.Model = tc.model + _, err := ResolveConfiguredModel(context.Background(), tc.cfg) + if err == nil { + t.Fatalf("%s: expected an error, got nil", tc.name) + } + if strings.Contains(err.Error(), "not usable with the current Pulse Assistant config") { + t.Fatalf("%s: error %q reached the supposedly-unreachable fallthrough", tc.name, err.Error()) + } + } +} diff --git a/internal/ai/qualification/ai_qualification_pure_branchcov0716_test.go b/internal/ai/qualification/ai_qualification_pure_branchcov0716_test.go new file mode 100644 index 000000000..4196ea77c --- /dev/null +++ b/internal/ai/qualification/ai_qualification_pure_branchcov0716_test.go @@ -0,0 +1,180 @@ +package qualification + +import ( + "errors" + "testing" +) + +// TestBranchCovNormalizeJSONText drives every behavioral edge of the +// normalizeJSONText helper. The helper is +// strings.Join(strings.Fields(strings.TrimSpace(value)), " "), so genuine +// branch coverage here means exercising empty input, whitespace-only input +// (covering each Unicode whitespace class that strings.Fields recognises: +// ASCII space/tab/newline/CR/VT/FF as well as non-ASCII space U+00A0), +// single-token input that is already normal, and inputs whose internal +// whitespace runs mix several separators that must all collapse to a single +// ASCII space. +func TestBranchCovNormalizeJSONText(t *testing.T) { + cases := []struct { + name string + input string + want string + }{ + {name: "empty string", input: "", want: ""}, + {name: "single ascii space", input: " ", want: ""}, + {name: "ascii whitespace only collapses to empty", input: " \t\n\r\v\f", want: ""}, + {name: "leading and trailing whitespace trimmed", input: " hello ", want: "hello"}, + {name: "already normal single token", input: "hello", want: "hello"}, + {name: "already normal two tokens", input: "hello world", want: "hello world"}, + {name: "multiple spaces collapsed", input: "hello world", want: "hello world"}, + {name: "mixed tab newline cr become single spaces", input: "a\tb\n c\r d", want: "a b c d"}, + {name: "newline separated lines flattened", input: "line1\nline2", want: "line1 line2"}, + {name: "non-ascii non-whitespace content preserved", input: "emoji 😀 text", want: "emoji 😀 text"}, + {name: "nbsp treated as field separator", input: "a\xc2\xa0b", want: "a b"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := normalizeJSONText(tc.input); got != tc.want { + t.Errorf("normalizeJSONText(%q) = %q, want %q", tc.input, got, tc.want) + } + }) + } +} + +// TestBranchCovPatrolAutonomyEffective covers both arms of the conditional in +// PatrolAutonomy.Effective: the EffectiveAutonomyLevel branch when that field +// has non-empty content after trimming (including that the result is itself +// trimmed), and the fallback arm when EffectiveAutonomyLevel is empty or +// whitespace-only (covering the AutonomyLevel trim on that path), plus the +// degenerate case where both fields are empty/whitespace. +func TestBranchCovPatrolAutonomyEffective(t *testing.T) { + cases := []struct { + name string + a PatrolAutonomy + want string + }{ + { + name: "effective level wins over autonomy level", + a: PatrolAutonomy{EffectiveAutonomyLevel: "auto", AutonomyLevel: "semi"}, + want: "auto", + }, + { + name: "effective level is trimmed before return", + a: PatrolAutonomy{EffectiveAutonomyLevel: " auto ", AutonomyLevel: "semi"}, + want: "auto", + }, + { + name: "empty effective falls back to autonomy level", + a: PatrolAutonomy{EffectiveAutonomyLevel: "", AutonomyLevel: "semi"}, + want: "semi", + }, + { + name: "whitespace-only effective falls back to autonomy level", + a: PatrolAutonomy{EffectiveAutonomyLevel: " ", AutonomyLevel: "semi"}, + want: "semi", + }, + { + name: "fallback autonomy level is trimmed", + a: PatrolAutonomy{EffectiveAutonomyLevel: "", AutonomyLevel: " semi "}, + want: "semi", + }, + { + name: "both fields whitespace only yields empty", + a: PatrolAutonomy{EffectiveAutonomyLevel: " ", AutonomyLevel: "\t"}, + want: "", + }, + {name: "zero value yields empty", a: PatrolAutonomy{}, want: ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.a.Effective(); got != tc.want { + t.Errorf("Effective() = %q, want %q", got, tc.want) + } + }) + } +} + +// TestBranchCovHTTPError asserts the real formatting behaviour of +// (*HTTPError).Error() across the meaningful input space: the zero value, a +// typical populated error, an empty body, a negative status code, and — +// crucially — the case where Body itself contains fmt verbs. Because Body is +// passed as an argument to fmt.Sprintf (not interpolated into the format +// string), such verbs must appear verbatim and never be re-expanded; this +// guards against a future refactor that turns the format into user-controlled +// text. +// +// Each case also exercises *HTTPError through the `error` interface, locking +// in that the method set still satisfies `error` (callers in client.go return +// &HTTPError{} as `error`, so a receiver change would silently break them). +func TestBranchCovHTTPError(t *testing.T) { + cases := []struct { + name string + err *HTTPError + want string + }{ + {name: "zero value", err: &HTTPError{}, want: "Pulse API returned 0: "}, + { + name: "typical 404", + err: &HTTPError{StatusCode: 404, Path: "/api/v1/foo", Body: "not found"}, + want: "Pulse API /api/v1/foo returned 404: not found", + }, + { + name: "empty body rendered with trailing colon space", + err: &HTTPError{StatusCode: 500, Path: "/api/v1/bar", Body: ""}, + want: "Pulse API /api/v1/bar returned 500: ", + }, + { + name: "negative status code rendered verbatim", + err: &HTTPError{StatusCode: -1, Path: "/x", Body: "weird"}, + want: "Pulse API /x returned -1: weird", + }, + { + name: "body containing fmt verbs is not re-expanded", + err: &HTTPError{StatusCode: 502, Path: "/api/v1/baz", Body: "boom %s %d %%"}, + want: "Pulse API /api/v1/baz returned 502: boom %s %d %%", + }, + { + name: "percent literal in path preserved", + err: &HTTPError{StatusCode: 400, Path: "/api/%2Fenc", Body: "bad"}, + want: "Pulse API /api/%2Fenc returned 400: bad", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var asErr error = tc.err + if got := asErr.Error(); got != tc.want { + t.Errorf("(*HTTPError).Error() = %q, want %q", got, tc.want) + } + }) + } +} + +// TestBranchCovHTTPErrorErrorsAsRoundTrip verifies the concrete type survives a +// round trip through the `error` interface via errors.As, mirroring how +// callers of client.go surface HTTP failures (and how they recover them with +// errors.As/Is). This locks in that *HTTPError — not HTTPError — is the +// addressable error type, so the errors.As target *HTTPError matches. +func TestBranchCovHTTPErrorErrorsAsRoundTrip(t *testing.T) { + original := &HTTPError{StatusCode: 418, Path: "/teapot", Body: "i am a teapot"} + var asErr error = original + + var target *HTTPError + if !errors.As(asErr, &target) { + t.Fatalf("errors.As((*HTTPError)(nil) target) = false, want true") + } + if target == nil { + t.Fatal("errors.As assigned nil target") + } + if target.StatusCode != original.StatusCode || + target.Path != original.Path || + target.Body != original.Body { + t.Errorf("errors.As round trip lost data: got %+v, want %+v", target, original) + } + + // An unrelated error type must NOT be misidentified as *HTTPError. + unrelated := errors.New("plain sentinel") + var badTarget *HTTPError + if errors.As(unrelated, &badTarget) { + t.Errorf("errors.As matched *HTTPError on unrelated error %v", unrelated) + } +} diff --git a/internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go b/internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go new file mode 100644 index 000000000..e28349796 --- /dev/null +++ b/internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go @@ -0,0 +1,1072 @@ +package config_test + +import ( + "reflect" + "testing" + + alertconfig "github.com/rcourtman/pulse-go-rewrite/internal/alerts/config" +) + +// This file adds branch-coverage tests for the per-subsystem Normalize*Defaults +// helpers in internal/alerts/config/normalize.go (NormalizePMGDefaults, +// NormalizePBSDefaults, NormalizeSnapshotDefaults, NormalizeBackupDefaults, +// NormalizeNodeDefaults, NormalizeAgentDefaults, NormalizeKubernetesDefaults, +// NormalizeTrueNASDefaults, NormalizeVMwareDefaults, NormalizeDiskTempByType, +// NormalizeMetricTimeThresholds) plus the unexported normalizeThresholdPointer. +// +// The unexported normalizeThresholdPointer is exercised indirectly through its +// exported callers (NormalizeKubernetesDefaults / NormalizeTrueNASDefaults / +// NormalizeVMwareDefaults) because the sibling test files use the external +// `package config_test` and so cannot reach unexported symbols directly. +// Test names use the BranchCov prefix so `-run BranchCov` selects them. +// +// floatEq / htEq are reused from alerts_config_normalize_branchcov0716_test.go. + +// ptrHtEq dereferences a *HysteresisThreshold and compares it by value. +func ptrHtEq(got *alertconfig.HysteresisThreshold, want alertconfig.HysteresisThreshold) bool { + if got == nil { + return false + } + return htEq(*got, want) +} + +// expectedPMGDefaults returns the canonical PMGThresholdConfig the normalizer +// must seed whenever a field is non-positive. +func expectedPMGDefaults() alertconfig.PMGThresholdConfig { + return alertconfig.PMGThresholdConfig{ + QueueTotalWarning: 500, + QueueTotalCritical: 1000, + OldestMessageWarnMins: 30, + OldestMessageCritMins: 60, + DeferredQueueWarn: 200, + DeferredQueueCritical: 500, + HoldQueueWarn: 100, + HoldQueueCritical: 300, + QuarantineSpamWarn: 2000, + QuarantineSpamCritical: 5000, + QuarantineVirusWarn: 2000, + QuarantineVirusCritical: 5000, + QuarantineGrowthWarnPct: 25, + QuarantineGrowthWarnMin: 250, + QuarantineGrowthCritPct: 50, + QuarantineGrowthCritMin: 500, + } +} + +// TestBranchCovNormalizePMGDefaults covers the <=0 default-seeding branch and +// the >0 preserve branch for every field of PMGThresholdConfig. +func TestBranchCovNormalizePMGDefaults(t *testing.T) { + t.Run("zero-valued config seeds every field with its canonical default", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizePMGDefaults(cfg) + if !reflect.DeepEqual(cfg.PMGDefaults, expectedPMGDefaults()) { + t.Fatalf("PMGDefaults = %+v, want defaults %+v", cfg.PMGDefaults, expectedPMGDefaults()) + } + }) + + t.Run("negative values are replaced with defaults (<=0 branch)", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + PMGDefaults: alertconfig.PMGThresholdConfig{ + QueueTotalWarning: -1, + QueueTotalCritical: -2, + OldestMessageWarnMins: -3, + OldestMessageCritMins: -4, + DeferredQueueWarn: -5, + DeferredQueueCritical: -6, + HoldQueueWarn: -7, + HoldQueueCritical: -8, + QuarantineSpamWarn: -9, + QuarantineSpamCritical: -10, + QuarantineVirusWarn: -11, + QuarantineVirusCritical: -12, + QuarantineGrowthWarnPct: -13, + QuarantineGrowthWarnMin: -14, + QuarantineGrowthCritPct: -15, + QuarantineGrowthCritMin: -16, + }, + } + alertconfig.NormalizePMGDefaults(cfg) + if !reflect.DeepEqual(cfg.PMGDefaults, expectedPMGDefaults()) { + t.Fatalf("PMGDefaults = %+v, want defaults (negatives replaced)", cfg.PMGDefaults) + } + }) + + t.Run("positive operator values are preserved (>0 branch)", func(t *testing.T) { + want := alertconfig.PMGThresholdConfig{ + QueueTotalWarning: 111, + QueueTotalCritical: 222, + OldestMessageWarnMins: 333, + OldestMessageCritMins: 444, + DeferredQueueWarn: 555, + DeferredQueueCritical: 666, + HoldQueueWarn: 777, + HoldQueueCritical: 888, + QuarantineSpamWarn: 999, + QuarantineSpamCritical: 1001, + QuarantineVirusWarn: 1002, + QuarantineVirusCritical: 1003, + QuarantineGrowthWarnPct: 12, + QuarantineGrowthWarnMin: 123, + QuarantineGrowthCritPct: 34, + QuarantineGrowthCritMin: 345, + } + cfg := &alertconfig.AlertConfig{PMGDefaults: want} + alertconfig.NormalizePMGDefaults(cfg) + if !reflect.DeepEqual(cfg.PMGDefaults, want) { + t.Fatalf("PMGDefaults = %+v, want %+v (preserved)", cfg.PMGDefaults, want) + } + }) + + t.Run("mixed zero/negative/positive fields only seed the non-positive ones", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + PMGDefaults: alertconfig.PMGThresholdConfig{ + QueueTotalWarning: 0, // -> 500 + QueueTotalCritical: 7777, // preserved + OldestMessageWarnMins: -5, // -> 30 + HoldQueueWarn: 42, // preserved + }, + } + alertconfig.NormalizePMGDefaults(cfg) + want := expectedPMGDefaults() + want.QueueTotalCritical = 7777 + want.HoldQueueWarn = 42 + if !reflect.DeepEqual(cfg.PMGDefaults, want) { + t.Fatalf("PMGDefaults = %+v, want %+v", cfg.PMGDefaults, want) + } + }) +} + +// TestBranchCovNormalizePBSDefaults covers each branch of the CPU/Memory ladder: +// nil/negative -> default pair, zero trigger -> clear 0, clear<=0 -> derive with a +// FIXED fallback, plus the valid no-op. It also pins the small-trigger anomaly. +func TestBranchCovNormalizePBSDefaults(t *testing.T) { + t.Run("nil thresholds are seeded with full default pairs", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizePBSDefaults(cfg) + if !ptrHtEq(cfg.PBSDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}) { + t.Fatalf("CPU = %+v, want {80 75}", cfg.PBSDefaults.CPU) + } + if !ptrHtEq(cfg.PBSDefaults.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}) { + t.Fatalf("Memory = %+v, want {85 80}", cfg.PBSDefaults.Memory) + } + }) + + // CPU defaults to {80,75}; Memory to {85,80}. Same input shape applied to + // both to cover every branch of the shared ladder. + tests := []struct { + name string + in alertconfig.HysteresisThreshold + wantCPU alertconfig.HysteresisThreshold + wantMemory alertconfig.HysteresisThreshold + }{ + { + name: "negative trigger replaced with default pair", + in: alertconfig.HysteresisThreshold{Trigger: -1, Clear: 99}, + wantCPU: alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + wantMemory: alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}, + }, + { + name: "zero trigger forces clear to zero", + in: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 50}, + wantCPU: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, + wantMemory: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + { + name: "positive trigger with zero clear derives clear as trigger-5", + in: alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + wantCPU: alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}, + wantMemory: alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}, + }, + { + // SUSPECTED SOURCE BUG: derived clear (1-5=-4) is <=0 so the code + // falls back to the FIXED default (75 CPU / 80 Memory), producing + // Clear >> Trigger. Unlike normalizeThresholdPointer (which clamps + // to 0) and with no EnsureValidHysteresis call, this stays invalid. + name: "small trigger with zero clear falls back to fixed default yielding clear>trigger", + in: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, + wantCPU: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}, + wantMemory: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 80}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cpu := tc.in + mem := tc.in + cfg := &alertconfig.AlertConfig{ + PBSDefaults: alertconfig.ThresholdConfig{ + CPU: &cpu, + Memory: &mem, + }, + } + alertconfig.NormalizePBSDefaults(cfg) + if !ptrHtEq(cfg.PBSDefaults.CPU, tc.wantCPU) { + t.Fatalf("CPU = %+v, want %+v", cfg.PBSDefaults.CPU, tc.wantCPU) + } + if !ptrHtEq(cfg.PBSDefaults.Memory, tc.wantMemory) { + t.Fatalf("Memory = %+v, want %+v", cfg.PBSDefaults.Memory, tc.wantMemory) + } + }) + } + + t.Run("already valid pairs are left untouched", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + PBSDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + Memory: &alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}, + }, + } + alertconfig.NormalizePBSDefaults(cfg) + if !ptrHtEq(cfg.PBSDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}) { + t.Fatalf("CPU = %+v, want unchanged", cfg.PBSDefaults.CPU) + } + if !ptrHtEq(cfg.PBSDefaults.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}) { + t.Fatalf("Memory = %+v, want unchanged", cfg.PBSDefaults.Memory) + } + }) +} + +// TestBranchCovNormalizeSnapshotDefaults covers the day and size clamps: negative +// to zero, warning>critical clamp-down, zero-critical promotion, and valid no-ops. +func TestBranchCovNormalizeSnapshotDefaults(t *testing.T) { + tests := []struct { + name string + warningDays int + criticalDays int + warningSizeGiB float64 + criticalSizeGiB float64 + wantWarnDays int + wantCritDays int + wantWarnSize float64 + wantCritSize float64 + }{ + { + name: "both days negative clamped to zero", + warningDays: -3, + criticalDays: -7, + wantWarnDays: 0, wantCritDays: 0, + }, + { + name: "warning days above positive critical clamped down to critical", + warningDays: 10, + criticalDays: 5, + wantWarnDays: 5, wantCritDays: 5, + }, + { + name: "zero critical with positive warning promotes critical to warning", + warningDays: 7, + criticalDays: 0, + wantWarnDays: 7, wantCritDays: 7, + }, + { + name: "valid days pair untouched", + warningDays: 3, + criticalDays: 7, + wantWarnDays: 3, wantCritDays: 7, + }, + { + name: "zero warning with positive critical untouched", + warningDays: 0, + criticalDays: 7, + wantWarnDays: 0, wantCritDays: 7, + }, + { + name: "both sizes negative clamped to zero", + warningSizeGiB: -2.5, + criticalSizeGiB: -9, + wantWarnSize: 0, wantCritSize: 0, + }, + { + name: "warning size above positive critical clamped down", + warningSizeGiB: 100, + criticalSizeGiB: 50, + wantWarnSize: 50, wantCritSize: 50, + }, + { + name: "zero critical size with positive warning promotes critical", + warningSizeGiB: 20, + criticalSizeGiB: 0, + wantWarnSize: 20, wantCritSize: 20, + }, + { + name: "valid size pair untouched", + warningSizeGiB: 10, + criticalSizeGiB: 30, + wantWarnSize: 10, wantCritSize: 30, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + SnapshotDefaults: alertconfig.SnapshotAlertConfig{ + WarningDays: tc.warningDays, + CriticalDays: tc.criticalDays, + WarningSizeGiB: tc.warningSizeGiB, + CriticalSizeGiB: tc.criticalSizeGiB, + }, + } + alertconfig.NormalizeSnapshotDefaults(cfg) + s := cfg.SnapshotDefaults + if s.WarningDays != tc.wantWarnDays { + t.Errorf("WarningDays = %d, want %d", s.WarningDays, tc.wantWarnDays) + } + if s.CriticalDays != tc.wantCritDays { + t.Errorf("CriticalDays = %d, want %d", s.CriticalDays, tc.wantCritDays) + } + if !floatEq(s.WarningSizeGiB, tc.wantWarnSize) { + t.Errorf("WarningSizeGiB = %v, want %v", s.WarningSizeGiB, tc.wantWarnSize) + } + if !floatEq(s.CriticalSizeGiB, tc.wantCritSize) { + t.Errorf("CriticalSizeGiB = %v, want %v", s.CriticalSizeGiB, tc.wantCritSize) + } + }) + } +} + +// TestBranchCovNormalizeBackupDefaults covers day clamps, FreshHours/StaleHours +// seeding, the StaleHours0)", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizeBackupDefaults(cfg) + if len(cfg.BackupDefaults.IgnoreVMIDs) != 0 { + t.Fatalf("IgnoreVMIDs = %#v, want empty", cfg.BackupDefaults.IgnoreVMIDs) + } + }) + + t.Run("IgnoreVMIDs with only empties collapses to empty slice", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + BackupDefaults: alertconfig.BackupAlertConfig{ + IgnoreVMIDs: []string{" ", "", "\t"}, + }, + } + alertconfig.NormalizeBackupDefaults(cfg) + if len(cfg.BackupDefaults.IgnoreVMIDs) != 0 { + t.Fatalf("IgnoreVMIDs = %#v, want empty", cfg.BackupDefaults.IgnoreVMIDs) + } + }) +} + +// TestBranchCovNormalizeNodeDefaults covers the Temperature pointer ladder: +// nil/negative -> default, zero trigger -> clear 0, clear<=0 -> derive with fixed +// fallback, valid no-op, and the small-trigger clear>trigger anomaly. +func TestBranchCovNormalizeNodeDefaults(t *testing.T) { + tests := []struct { + name string + in *alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + { + name: "nil temperature seeded with default", + in: nil, + want: alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + }, + { + name: "negative trigger replaced with default", + in: &alertconfig.HysteresisThreshold{Trigger: -1, Clear: 99}, + want: alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + }, + { + name: "zero trigger forces clear to zero", + in: &alertconfig.HysteresisThreshold{Trigger: 0, Clear: 50}, + want: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + { + name: "positive trigger zero clear derives clear", + in: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + want: alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}, + }, + { + // SUSPECTED SOURCE BUG: derived clear (1-5=-4) <=0 falls back to the + // FIXED 75, giving Clear(75) > Trigger(1); NormalizeNodeDefaults has + // no EnsureValidHysteresis call so this stays invalid. + name: "small trigger zero clear falls back to fixed 75 (clear>trigger)", + in: &alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, + want: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}, + }, + { + name: "valid pair untouched", + in: &alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + want: alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + NodeDefaults: alertconfig.ThresholdConfig{Temperature: tc.in}, + } + alertconfig.NormalizeNodeDefaults(cfg) + if !ptrHtEq(cfg.NodeDefaults.Temperature, tc.want) { + t.Fatalf("Temperature = %+v, want %+v", cfg.NodeDefaults.Temperature, tc.want) + } + }) + } +} + +// TestBranchCovNormalizeAgentDefaults covers CPU/Memory/Disk/DiskTemperature +// seeding, negative/zero-trigger branches, clear derivation, the +// EnsureValidHysteresis safety net on DiskTemperature (incl. the small-trigger +// fallback repaired back to 0), and the side-effect seeding of both disk maps. +func TestBranchCovNormalizeAgentDefaults(t *testing.T) { + t.Run("nil thresholds seeded and both disk type maps populated", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizeAgentDefaults(cfg) + a := cfg.AgentDefaults + if !ptrHtEq(a.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}) { + t.Fatalf("CPU = %+v, want {80 75}", a.CPU) + } + if !ptrHtEq(a.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}) { + t.Fatalf("Memory = %+v, want {85 80}", a.Memory) + } + if !ptrHtEq(a.Disk, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("Disk = %+v, want {90 85}", a.Disk) + } + if !ptrHtEq(a.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 55, Clear: 50}) { + t.Fatalf("DiskTemperature = %+v, want {55 50}", a.DiskTemperature) + } + if len(cfg.DiskFillByType) != 3 { + t.Fatalf("DiskFillByType len = %d, want 3", len(cfg.DiskFillByType)) + } + if len(cfg.DiskTempByType) != 3 { + t.Fatalf("DiskTempByType len = %d, want 3", len(cfg.DiskTempByType)) + } + }) + + t.Run("negative triggers replaced with defaults", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: -1, Clear: 0}, + Memory: &alertconfig.HysteresisThreshold{Trigger: -1, Clear: 0}, + Disk: &alertconfig.HysteresisThreshold{Trigger: -1, Clear: 0}, + DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: -1, Clear: 0}, + }, + } + alertconfig.NormalizeAgentDefaults(cfg) + a := cfg.AgentDefaults + if !ptrHtEq(a.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}) { + t.Fatalf("CPU = %+v, want {80 75}", a.CPU) + } + if !ptrHtEq(a.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}) { + t.Fatalf("Memory = %+v, want {85 80}", a.Memory) + } + if !ptrHtEq(a.Disk, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("Disk = %+v, want {90 85}", a.Disk) + } + if !ptrHtEq(a.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 55, Clear: 50}) { + t.Fatalf("DiskTemperature = %+v, want {55 50}", a.DiskTemperature) + } + }) + + t.Run("zero trigger forces clear to zero for every threshold", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 0, Clear: 99}, + Memory: &alertconfig.HysteresisThreshold{Trigger: 0, Clear: 99}, + Disk: &alertconfig.HysteresisThreshold{Trigger: 0, Clear: 99}, + DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: 0, Clear: 99}, + }, + } + alertconfig.NormalizeAgentDefaults(cfg) + zero := alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0} + a := cfg.AgentDefaults + if !ptrHtEq(a.CPU, zero) || !ptrHtEq(a.Memory, zero) || !ptrHtEq(a.Disk, zero) || !ptrHtEq(a.DiskTemperature, zero) { + t.Fatalf("expected all zero, got CPU=%+v Mem=%+v Disk=%+v DiskTemp=%+v", a.CPU, a.Memory, a.Disk, a.DiskTemperature) + } + }) + + t.Run("positive trigger with non-positive clear derives clear for each threshold", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + Memory: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + Disk: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: 60, Clear: 0}, + }, + } + alertconfig.NormalizeAgentDefaults(cfg) + a := cfg.AgentDefaults + if !ptrHtEq(a.CPU, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("CPU = %+v, want {90 85}", a.CPU) + } + if !ptrHtEq(a.Memory, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("Memory = %+v, want {90 85}", a.Memory) + } + if !ptrHtEq(a.Disk, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("Disk = %+v, want {90 85}", a.Disk) + } + if !ptrHtEq(a.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 60, Clear: 55}) { + t.Fatalf("DiskTemperature = %+v, want {60 55}", a.DiskTemperature) + } + }) + + // SUSPECTED SOURCE BUG (CPU arm): derived clear (1-5=-4) <=0 falls back to + // the FIXED 75; CPU has NO EnsureValidHysteresis call, so Clear(75) > + // Trigger(1) persists. DiskTemperature (which DOES call EnsureValidHysteresis) + // is repaired - covered by the next two subtests. + t.Run("CPU small trigger zero clear yields clear>trigger (no EnsureValidHysteresis net)", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, + }, + } + alertconfig.NormalizeAgentDefaults(cfg) + if !ptrHtEq(cfg.AgentDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}) { + t.Fatalf("CPU = %+v, want {1 75} (clear>trigger anomaly)", cfg.AgentDefaults.CPU) + } + }) + + t.Run("DiskTemperature invalid clear>=trigger repaired by EnsureValidHysteresis", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: 55, Clear: 70}, + }, + } + alertconfig.NormalizeAgentDefaults(cfg) + // Clear(70) > 0 so derive is skipped; EnsureValidHysteresis repairs 70>=55 -> 50. + if !ptrHtEq(cfg.AgentDefaults.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 55, Clear: 50}) { + t.Fatalf("DiskTemperature = %+v, want {55 50}", cfg.AgentDefaults.DiskTemperature) + } + }) + + t.Run("DiskTemperature small trigger: fallback to 50 then repaired to 0 by EnsureValidHysteresis", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: 2, Clear: 0}, + }, + } + alertconfig.NormalizeAgentDefaults(cfg) + // derived clear 2-5=-3 <=0 -> fixed fallback 50; EnsureValidHysteresis + // then sees 50>=2 and sets clear=2-5=-3<0 -> 0. End state {2 0}. + if !ptrHtEq(cfg.AgentDefaults.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 2, Clear: 0}) { + t.Fatalf("DiskTemperature = %+v, want {2 0} (fallback 50 repaired to 0)", cfg.AgentDefaults.DiskTemperature) + } + }) + + t.Run("valid thresholds preserved end to end", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + Memory: &alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}, + Disk: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}, + DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: 55, Clear: 50}, + }, + } + alertconfig.NormalizeAgentDefaults(cfg) + a := cfg.AgentDefaults + if !ptrHtEq(a.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}) { + t.Fatalf("CPU = %+v, want unchanged", a.CPU) + } + if !ptrHtEq(a.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 55, Clear: 50}) { + t.Fatalf("DiskTemperature = %+v, want unchanged", a.DiskTemperature) + } + }) +} + +// TestBranchCovNormalizeKubernetesDefaults exercises normalizeThresholdPointer's +// full branch ladder through CPU (defaults 80/75) and DiskRead (defaults 0/0), +// and confirms every field becomes a non-nil pointer after a zero config. +func TestBranchCovNormalizeKubernetesDefaults(t *testing.T) { + t.Run("all nil: CPU/Memory/Disk seeded, IO and network fields are non-nil {0,0}", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizeKubernetesDefaults(cfg) + k := cfg.KubernetesDefaults + for _, f := range []struct { + name string + got *alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + {"CPU", k.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}}, + {"Memory", k.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}}, + {"Disk", k.Disk, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}}, + {"DiskRead", k.DiskRead, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"DiskWrite", k.DiskWrite, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"NetworkIn", k.NetworkIn, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"NetworkOut", k.NetworkOut, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + } { + if f.got == nil { + t.Fatalf("%s is nil, want non-nil", f.name) + } + if !htEq(*f.got, f.want) { + t.Fatalf("%s = %+v, want %+v", f.name, *f.got, f.want) + } + } + }) + + // normalizeThresholdPointer branch ladder via CPU (defaultTrigger 80, defaultClear 75). + t.Run("CPU branch matrix (defaults 80/75)", func(t *testing.T) { + tests := []struct { + name string + in *alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + { + name: "negative trigger returns default pair", + in: &alertconfig.HysteresisThreshold{Trigger: -1, Clear: 99}, + want: alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + }, + { + name: "zero trigger zeroes clear and returns a copy", + in: &alertconfig.HysteresisThreshold{Trigger: 0, Clear: 50}, + want: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + { + name: "positive trigger zero clear derives clear", + in: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + want: alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}, + }, + { + // Contrast with PBS/Node/Agent: derived clear clamps to 0 here, + // NOT a fixed fallback default, so no clear>trigger anomaly. + name: "derived clear clamps to zero (not fixed fallback) when trigger<5", + in: &alertconfig.HysteresisThreshold{Trigger: 3, Clear: 0}, + want: alertconfig.HysteresisThreshold{Trigger: 3, Clear: 0}, + }, + { + name: "clear above trigger repaired by EnsureValidHysteresis", + in: &alertconfig.HysteresisThreshold{Trigger: 80, Clear: 90}, + want: alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + }, + { + name: "valid pair untouched", + in: &alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + want: alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + in := *tc.in + cfg := &alertconfig.AlertConfig{ + KubernetesDefaults: alertconfig.ThresholdConfig{CPU: &in}, + } + alertconfig.NormalizeKubernetesDefaults(cfg) + if !ptrHtEq(cfg.KubernetesDefaults.CPU, tc.want) { + t.Fatalf("CPU = %+v, want %+v", cfg.KubernetesDefaults.CPU, tc.want) + } + }) + } + }) + + // DiskRead uses defaults (0,0): nil yields a non-nil {0,0} (disabled), but an + // operator-provided positive trigger enables the metric and derives a clear. + t.Run("DiskRead branch matrix (defaults 0/0)", func(t *testing.T) { + tests := []struct { + name string + in *alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + { + name: "nil returns non-nil {0,0} (disabled)", + in: nil, + want: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + { + name: "negative trigger returns {0,0}", + in: &alertconfig.HysteresisThreshold{Trigger: -5, Clear: 99}, + want: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + { + name: "zero trigger zeroes clear", + in: &alertconfig.HysteresisThreshold{Trigger: 0, Clear: 99}, + want: alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + { + name: "positive trigger enables metric and derives clear", + in: &alertconfig.HysteresisThreshold{Trigger: 50, Clear: 0}, + want: alertconfig.HysteresisThreshold{Trigger: 50, Clear: 45}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + KubernetesDefaults: alertconfig.ThresholdConfig{DiskRead: tc.in}, + } + alertconfig.NormalizeKubernetesDefaults(cfg) + if cfg.KubernetesDefaults.DiskRead == nil { + t.Fatalf("DiskRead nil, want non-nil") + } + if !htEq(*cfg.KubernetesDefaults.DiskRead, tc.want) { + t.Fatalf("DiskRead = %+v, want %+v", *cfg.KubernetesDefaults.DiskRead, tc.want) + } + }) + } + }) +} + +// TestBranchCovNormalizeTrueNASDefaults covers the full TrueNAS seed matrix, +// operator-enabled IO metrics, and the EnsureValidHysteresis repair path. +func TestBranchCovNormalizeTrueNASDefaults(t *testing.T) { + t.Run("nil config seeds every threshold with its subsystem default", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizeTrueNASDefaults(cfg) + t1 := cfg.TrueNASDefaults + for _, f := range []struct { + name string + got *alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + {"CPU", t1.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}}, + {"Memory", t1.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}}, + {"Disk", t1.Disk, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}}, + {"Usage", t1.Usage, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}}, + {"Temperature", t1.Temperature, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}}, + {"DiskRead", t1.DiskRead, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"DiskWrite", t1.DiskWrite, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"NetworkIn", t1.NetworkIn, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"NetworkOut", t1.NetworkOut, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"Disk.Temperature", cfg.TrueNASDiskDefaults.Temperature, alertconfig.HysteresisThreshold{Trigger: 55, Clear: 50}}, + } { + if f.got == nil { + t.Fatalf("%s is nil, want non-nil", f.name) + } + if !htEq(*f.got, f.want) { + t.Fatalf("%s = %+v, want %+v", f.name, *f.got, f.want) + } + } + }) + + t.Run("derivation and operator-enabled IO metric via CPU/DiskRead", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + TrueNASDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + DiskRead: &alertconfig.HysteresisThreshold{Trigger: 40, Clear: 0}, + }, + } + alertconfig.NormalizeTrueNASDefaults(cfg) + if !ptrHtEq(cfg.TrueNASDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("CPU = %+v, want {90 85}", cfg.TrueNASDefaults.CPU) + } + if !ptrHtEq(cfg.TrueNASDefaults.DiskRead, alertconfig.HysteresisThreshold{Trigger: 40, Clear: 35}) { + t.Fatalf("DiskRead = %+v, want {40 35}", cfg.TrueNASDefaults.DiskRead) + } + }) + + t.Run("EnsureValidHysteresis repairs clear>=trigger", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + TrueNASDefaults: alertconfig.ThresholdConfig{ + Memory: &alertconfig.HysteresisThreshold{Trigger: 85, Clear: 95}, + }, + } + alertconfig.NormalizeTrueNASDefaults(cfg) + if !ptrHtEq(cfg.TrueNASDefaults.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}) { + t.Fatalf("Memory = %+v, want {85 80}", cfg.TrueNASDefaults.Memory) + } + }) +} + +// TestBranchCovNormalizeVMwareDefaults covers the full VMware seed matrix plus +// derivation, an operator-enabled network metric, and EnsureValidHysteresis repair. +func TestBranchCovNormalizeVMwareDefaults(t *testing.T) { + t.Run("nil config seeds every threshold with its subsystem default", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizeVMwareDefaults(cfg) + v := cfg.VMwareDefaults + for _, f := range []struct { + name string + got *alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + {"CPU", v.CPU, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}}, + {"Memory", v.Memory, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}}, + {"Disk", v.Disk, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}}, + {"Usage", v.Usage, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}}, + {"DiskRead", v.DiskRead, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"DiskWrite", v.DiskWrite, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"NetworkIn", v.NetworkIn, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + {"NetworkOut", v.NetworkOut, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + } { + if f.got == nil { + t.Fatalf("%s is nil, want non-nil", f.name) + } + if !htEq(*f.got, f.want) { + t.Fatalf("%s = %+v, want %+v", f.name, *f.got, f.want) + } + } + }) + + t.Run("derivation, operator-enabled network metric, and EnsureValidHysteresis repair", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + VMwareDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 0}, + Disk: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 99}, + NetworkOut: &alertconfig.HysteresisThreshold{Trigger: 30, Clear: 0}, + }, + } + alertconfig.NormalizeVMwareDefaults(cfg) + if !ptrHtEq(cfg.VMwareDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("CPU = %+v, want {90 85}", cfg.VMwareDefaults.CPU) + } + // Disk clear(99)>=trigger(90) repaired by EnsureValidHysteresis -> 85. + if !ptrHtEq(cfg.VMwareDefaults.Disk, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { + t.Fatalf("Disk = %+v, want {90 85} (repaired)", cfg.VMwareDefaults.Disk) + } + // NetworkOut (0,0 default) operator-enabled: 30 -> clear 25. + if !ptrHtEq(cfg.VMwareDefaults.NetworkOut, alertconfig.HysteresisThreshold{Trigger: 30, Clear: 25}) { + t.Fatalf("NetworkOut = %+v, want {30 25}", cfg.VMwareDefaults.NetworkOut) + } + }) +} + +// TestBranchCovNormalizeDiskTempByType covers nil seeding, whitespace-key drop, +// canonical backfill, key lowercasing (incl. non-overwrite of existing canonical +// and value-preserving rename into an absent slot), and the non-positive reset. +func TestBranchCovNormalizeDiskTempByType(t *testing.T) { + wantDefaults := map[string]alertconfig.HysteresisThreshold{ + "nvme": {Trigger: 70, Clear: 65}, + "sas": {Trigger: 65, Clear: 60}, + "sata": {Trigger: 55, Clear: 50}, + } + + t.Run("nil map seeds canonical nvme/sas/sata defaults", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{} + alertconfig.NormalizeDiskTempByType(cfg) + if !reflect.DeepEqual(cfg.DiskTempByType, wantDefaults) { + t.Fatalf("DiskTempByType = %+v, want %+v", cfg.DiskTempByType, wantDefaults) + } + }) + + t.Run("whitespace-only key is dropped and canonical keys backfilled", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + DiskTempByType: map[string]alertconfig.HysteresisThreshold{ + " ": {Trigger: 99, Clear: 99}, + }, + } + alertconfig.NormalizeDiskTempByType(cfg) + if _, ok := cfg.DiskTempByType[" "]; ok { + t.Fatalf("whitespace key should be removed, map=%+v", cfg.DiskTempByType) + } + if _, ok := cfg.DiskTempByType[""]; ok { + t.Fatalf("empty key should not be added, map=%+v", cfg.DiskTempByType) + } + for _, k := range []string{"nvme", "sas", "sata"} { + if _, ok := cfg.DiskTempByType[k]; !ok { + t.Fatalf("canonical key %q missing, map=%+v", k, cfg.DiskTempByType) + } + } + }) + + t.Run("non-canonical already-lowercase key is preserved", func(t *testing.T) { + custom := alertconfig.HysteresisThreshold{Trigger: 40, Clear: 35} + cfg := &alertconfig.AlertConfig{ + DiskTempByType: map[string]alertconfig.HysteresisThreshold{ + "external": custom, + }, + } + alertconfig.NormalizeDiskTempByType(cfg) + got, ok := cfg.DiskTempByType["external"] + if !ok { + t.Fatalf("external key should survive, map=%+v", cfg.DiskTempByType) + } + if !htEq(got, custom) { + t.Fatalf("external = %+v, want %+v preserved", got, custom) + } + }) + + t.Run("uppercased duplicate of canonical key does not overwrite existing", func(t *testing.T) { + original := alertconfig.HysteresisThreshold{Trigger: 50, Clear: 45} + cfg := &alertconfig.AlertConfig{ + DiskTempByType: map[string]alertconfig.HysteresisThreshold{ + "SAS": {Trigger: 1, Clear: 1}, + "sas": original, + }, + } + alertconfig.NormalizeDiskTempByType(cfg) + if _, ok := cfg.DiskTempByType["SAS"]; ok { + t.Fatalf("uppercase SAS should be removed, map=%+v", cfg.DiskTempByType) + } + if got := cfg.DiskTempByType["sas"]; !htEq(got, original) { + t.Fatalf("sas = %+v, want %+v (not overwritten by dup)", got, original) + } + }) + + t.Run("mixed-case key lowercased into absent canonical slot keeps its value", func(t *testing.T) { + custom := alertconfig.HysteresisThreshold{Trigger: 72, Clear: 67} + cfg := &alertconfig.AlertConfig{ + DiskTempByType: map[string]alertconfig.HysteresisThreshold{ + "NVMe": custom, + }, + } + alertconfig.NormalizeDiskTempByType(cfg) + if _, ok := cfg.DiskTempByType["NVMe"]; ok { + t.Fatalf("mixed-case NVMe should be removed, map=%+v", cfg.DiskTempByType) + } + got, ok := cfg.DiskTempByType["nvme"] + if !ok { + t.Fatalf("lowercased nvme should be present, map=%+v", cfg.DiskTempByType) + } + if !htEq(got, custom) { + t.Fatalf("nvme = %+v, want %+v (value preserved through rename)", got, custom) + } + }) + + t.Run("non-positive trigger or clear resets canonical key to default", func(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + DiskTempByType: map[string]alertconfig.HysteresisThreshold{ + "sata": {Trigger: 88, Clear: 0}, + "sas": {Trigger: 0, Clear: 60}, + "nvme": {Trigger: 70, Clear: -1}, + }, + } + alertconfig.NormalizeDiskTempByType(cfg) + if got := cfg.DiskTempByType["sata"]; !htEq(got, wantDefaults["sata"]) { + t.Fatalf("sata = %+v, want default %+v (clear<=0)", got, wantDefaults["sata"]) + } + if got := cfg.DiskTempByType["sas"]; !htEq(got, wantDefaults["sas"]) { + t.Fatalf("sas = %+v, want default %+v (trigger<=0)", got, wantDefaults["sas"]) + } + if got := cfg.DiskTempByType["nvme"]; !htEq(got, wantDefaults["nvme"]) { + t.Fatalf("nvme = %+v, want default %+v (clear<=0)", got, wantDefaults["nvme"]) + } + }) +} + +// TestBranchCovNormalizeMetricTimeThresholds covers the empty->nil early returns, +// type canonicalization, legacy-type rejection (with "all" preserved), metric +// trim/lowercase, negative-delay and empty-metric dropping, and the full-filter +// collapse to nil. +func TestBranchCovNormalizeMetricTimeThresholds(t *testing.T) { + tests := []struct { + name string + in map[string]map[string]int + want map[string]map[string]int + }{ + {name: "nil input returns nil", in: nil, want: nil}, + {name: "empty input returns nil", in: map[string]map[string]int{}, want: nil}, + {name: "whitespace-only type key dropped -> nil", in: map[string]map[string]int{" ": {"cpu": 5}}, want: nil}, + {name: "empty inner metrics map dropped -> nil", in: map[string]map[string]int{"guest": {}}, want: nil}, + {name: "unsupported legacy type qemu dropped -> nil", in: map[string]map[string]int{"qemu": {"cpu": 5}}, want: nil}, + {name: "unsupported legacy type docker dropped -> nil", in: map[string]map[string]int{"docker": {"cpu": 5}}, want: nil}, + { + name: "all type preserved through the legacy guard", + in: map[string]map[string]int{"all": {"cpu": 7}}, + want: map[string]map[string]int{"all": {"cpu": 7}}, + }, + { + name: "type canonicalized (Kubernetes Pod -> pod) and metric trimmed/lowercased", + in: map[string]map[string]int{"Kubernetes Pod": {" CPU ": 10}}, + want: map[string]map[string]int{"pod": {"cpu": 10}}, + }, + { + name: "type trimmed/lowercased ( Guest -> guest)", + in: map[string]map[string]int{" Guest ": {"Disk": 4}}, + want: map[string]map[string]int{"guest": {"disk": 4}}, + }, + { + name: "empty metric key and negative delay are dropped, valid kept", + in: map[string]map[string]int{"guest": {"": 5, "cpu": -1, "mem": 3}}, + want: map[string]map[string]int{"guest": {"mem": 3}}, + }, + { + name: "everything filtered out collapses to nil", + in: map[string]map[string]int{"guest": {"": 1}, "qemu": {"cpu": 2}}, + want: nil, + }, + { + name: "multiple valid types normalized independently", + in: map[string]map[string]int{"node": {"CPU": 6}, "guest": {"memory": 8, "disk": 9}}, + want: map[string]map[string]int{"node": {"cpu": 6}, "guest": {"memory": 8, "disk": 9}}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := alertconfig.NormalizeMetricTimeThresholds(tc.in) + if tc.want == nil { + if got != nil { + t.Fatalf("got %+v, want nil", got) + } + return + } + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("got %+v, want %+v", got, tc.want) + } + }) + } +} diff --git a/internal/monitoring/monitoring_fleet_doctor_helpers_branchcov0716_test.go b/internal/monitoring/monitoring_fleet_doctor_helpers_branchcov0716_test.go new file mode 100644 index 000000000..d01a2db75 --- /dev/null +++ b/internal/monitoring/monitoring_fleet_doctor_helpers_branchcov0716_test.go @@ -0,0 +1,318 @@ +package monitoring + +import ( + "testing" + "time" +) + +// This file adds branch-coverage tests for the small pure helper functions in +// agent_fleet_doctor.go: +// - identitySplitReason +// - sameAgentIdentity +// - hasType +// - nonEmptyStrings +// - roundDuration +// +// It targets genuinely-uncovered branches (both sides of each conditional, +// short-circuit ordering, empty/nil/zero and boundary inputs) and asserts real +// behaviour using only the package's own (same-package, unexported) functions. +// Tests are prefixed with BranchCov so `-run BranchCov` selects them. + +// identitySplitReason always emits the two base evidence lines ("Peer type:" +// and "Peer ID:") and conditionally appends agent/token lines only when those +// values are non-empty. The returned Code/Severity/Message are invariant. +func TestBranchCovIdentitySplitReason(t *testing.T) { + t.Parallel() + + const wantCode = "agent_identity_split" + const wantSeverity = AgentFleetStatusWarning + const wantMessage = "Host and workload telemetry appear to belong to the same machine but are reporting as separate agent identities." + + cases := []struct { + name string + peerType string + peerID string + peerAgentID string + peerTokenID string + wantEvidenceLength int + }{ + // Both optional branches skipped: only the two base evidence lines. + {name: "both ids empty -> base evidence only", peerType: "Docker", peerID: "docker-1", peerAgentID: "", peerTokenID: "", wantEvidenceLength: 2}, + + // Only the agent-id branch taken. + {name: "only agent id present", peerType: "Host", peerID: "host-1", peerAgentID: "agent-9", peerTokenID: "", wantEvidenceLength: 3}, + + // Only the token-id branch taken (independent of the agent-id branch). + {name: "only token id present", peerType: "Docker", peerID: "docker-2", peerAgentID: "", peerTokenID: "tok-3", wantEvidenceLength: 3}, + + // Both optional branches taken. + {name: "both ids present", peerType: "Host", peerID: "host-2", peerAgentID: "agent-1", peerTokenID: "tok-1", wantEvidenceLength: 4}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + reason := identitySplitReason(tc.peerType, tc.peerID, tc.peerAgentID, tc.peerTokenID) + + if reason.Code != wantCode { + t.Fatalf("Code = %q, want %q", reason.Code, wantCode) + } + if reason.Severity != wantSeverity { + t.Fatalf("Severity = %q, want %q", reason.Severity, wantSeverity) + } + if reason.Message != wantMessage { + t.Fatalf("Message = %q, want %q", reason.Message, wantMessage) + } + + // The two base evidence lines are always present and carry the + // supplied peerType/peerID verbatim. + if len(reason.Evidence) != tc.wantEvidenceLength { + t.Fatalf("len(Evidence) = %d, want %d (%#v)", len(reason.Evidence), tc.wantEvidenceLength, reason.Evidence) + } + if got := reason.Evidence[0]; got != "Peer type: "+tc.peerType { + t.Fatalf("Evidence[0] = %q, want %q", got, "Peer type: "+tc.peerType) + } + if got := reason.Evidence[1]; got != "Peer ID: "+tc.peerID { + t.Fatalf("Evidence[1] = %q, want %q", got, "Peer ID: "+tc.peerID) + } + + // When the optional IDs are supplied they must be appended, in + // order, after the base lines. + offset := 2 + if tc.peerAgentID != "" { + if got := reason.Evidence[offset]; got != "Peer agent ID: "+tc.peerAgentID { + t.Fatalf("agent evidence = %q, want %q", got, "Peer agent ID: "+tc.peerAgentID) + } + offset++ + } + if tc.peerTokenID != "" { + if got := reason.Evidence[offset]; got != "Peer token ID: "+tc.peerTokenID { + t.Fatalf("token evidence = %q, want %q", got, "Peer token ID: "+tc.peerTokenID) + } + } + }) + } +} + +// TestBranchCovIdentitySplitReasonEmptyBaseLines documents real behaviour: the +// base peerType/peerID evidence lines are emitted unconditionally even when +// those values are empty, producing "Peer type: " / "Peer ID: " entries. +func TestBranchCovIdentitySplitReasonEmptyBaseLines(t *testing.T) { + t.Parallel() + + reason := identitySplitReason("", "", "", "") + if len(reason.Evidence) != 2 { + t.Fatalf("len(Evidence) = %d, want 2 (%#v)", len(reason.Evidence), reason.Evidence) + } + if reason.Evidence[0] != "Peer type: " || reason.Evidence[1] != "Peer ID: " { + t.Fatalf("empty base evidence = %#v, want [\"Peer type: \" \"Peer ID: \"]", reason.Evidence) + } +} + +// sameAgentIdentity short-circuits through three independent identity signals +// (agentID, tokenID, hostname). These cases pin down each branch, the +// short-circuit priority, the empty-guard fall-throughs, and the +// case-insensitive hostname comparison. +func TestBranchCovSameAgentIdentity(t *testing.T) { + t.Parallel() + + mkSubject := func(agentID, tokenID, hostname string) agentFleetSubject { + return agentFleetSubject{agentID: agentID, tokenID: tokenID, hostname: hostname} + } + + cases := []struct { + name string + subject agentFleetSubject + id string + agentID string + hostname string + tokenID string + want bool + descriptionOfHit string + }{ + // agentID branch: match via the agentID argument. + {name: "agent id matches agentID arg", subject: mkSubject("a1", "", ""), id: "x", agentID: "a1", hostname: "", tokenID: "", want: true}, + + // agentID branch: match via the id argument (the second operand of the + // short-circuit OR), agentID arg deliberately differs. + {name: "agent id matches id arg", subject: mkSubject("a1", "", ""), id: "a1", agentID: "other", hostname: "", tokenID: "", want: true}, + + // agentID guard: subject.agentID empty -> falls through to token/hostname. + {name: "subject agentID empty falls through to false", subject: mkSubject("", "", ""), id: "x", agentID: "y", hostname: "", tokenID: "", want: false}, + + // agentID guard: subject.agentID set but matches neither arg -> falls + // through to token, which then matches. + {name: "agentID no match falls to token match", subject: mkSubject("a1", "t1", ""), id: "x", agentID: "y", hostname: "", tokenID: "t1", want: true}, + + // tokenID branch: match when subject & peer both non-empty and equal. + {name: "token id match", subject: mkSubject("", "t1", ""), id: "", agentID: "", hostname: "", tokenID: "t1", want: true}, + + // tokenID guard: subject.tokenID empty -> skip (hostname empty -> false). + {name: "subject token empty skip -> false", subject: mkSubject("", "", ""), id: "", agentID: "", hostname: "h", tokenID: "t1", want: false}, + + // tokenID guard: peer tokenID empty -> skip even when subject token set, + // then fall through to a hostname match. + {name: "peer token empty skip -> falls to hostname match", subject: mkSubject("", "t1", "h"), id: "", agentID: "", hostname: "h", tokenID: "", want: true}, + + // tokenID guard: peer tokenID empty -> skip and subject has no hostname + // to fall through to, so overall false. + {name: "peer token empty skip -> no hostname -> false", subject: mkSubject("", "t1", ""), id: "", agentID: "", hostname: "h", tokenID: "", want: false}, + + // tokenID comparison: both non-empty but unequal -> skip. + {name: "tokens differ skip -> false", subject: mkSubject("", "t1", ""), id: "", agentID: "", hostname: "", tokenID: "t2", want: false}, + + // hostname branch: case-insensitive match. + {name: "hostname case-insensitive match", subject: mkSubject("", "", "Host-A"), id: "", agentID: "", hostname: "host-a", tokenID: "", want: true}, + + // hostname guard: subject.hostname empty -> skip -> false. + {name: "subject hostname empty -> false", subject: mkSubject("", "", ""), id: "", agentID: "", hostname: "host-a", tokenID: "", want: false}, + + // hostname comparison: peer hostname empty while subject set -> + // EqualFold returns false -> overall false. + {name: "peer hostname empty subject set -> false", subject: mkSubject("", "", "host-a"), id: "", agentID: "", hostname: "", tokenID: "", want: false}, + + // All signals absent -> false. + {name: "all empty -> false", subject: mkSubject("", "", ""), id: "", agentID: "", hostname: "", tokenID: "", want: false}, + + // Short-circuit priority: agentID match wins even when token and + // hostname would disagree. + {name: "agentID match priority ignores token/hostname mismatch", subject: mkSubject("a1", "t1", "real-host"), id: "x", agentID: "a1", hostname: "different", tokenID: "tX", want: true}, + + // Short-circuit priority: token match wins even when hostname would + // disagree (agentID empty so token branch is reached). + {name: "token match priority ignores hostname mismatch", subject: mkSubject("", "t1", "real-host"), id: "", agentID: "", hostname: "different", tokenID: "t1", want: true}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := sameAgentIdentity(tc.subject, tc.id, tc.agentID, tc.hostname, tc.tokenID) + if got != tc.want { + t.Fatalf("sameAgentIdentity(%+v, id=%q, agentID=%q, hostname=%q, tokenID=%q) = %v, want %v", + tc.subject, tc.id, tc.agentID, tc.hostname, tc.tokenID, got, tc.want) + } + }) + } +} + +// hasType is a thin map-membership check. Cover present, absent, nil-map and +// empty-string-key edges (a nil map lookup is well-defined in Go and returns +// ok=false). +func TestBranchCovHasType(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + types map[string]struct{} + kind string + want bool + }{ + {name: "present key -> true", types: map[string]struct{}{"docker": {}}, kind: "docker", want: true}, + {name: "absent key -> false", types: map[string]struct{}{"host": {}}, kind: "docker", want: false}, + {name: "nil map -> false", types: nil, kind: "docker", want: false}, + {name: "empty map -> false", types: map[string]struct{}{}, kind: "docker", want: false}, + {name: "empty key present -> true", types: map[string]struct{}{"": {}}, kind: "", want: true}, + {name: "empty key absent -> false", types: map[string]struct{}{"docker": {}}, kind: "", want: false}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if got := hasType(tc.types, tc.kind); got != tc.want { + t.Fatalf("hasType(%#v, %q) = %v, want %v", tc.types, tc.kind, got, tc.want) + } + }) + } +} + +// nonEmptyStrings trims whitespace from each arg and drops empties. It always +// returns a non-nil slice (it is initialised with make), including for a +// zero-arg / all-empty invocation. +func TestBranchCovNonEmptyStrings(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + input []string + want []string + wantOK bool // always true: result must be non-nil + }{ + {name: "no args -> non-nil empty", input: nil, want: []string{}, wantOK: true}, + {name: "all empty/whitespace -> non-nil empty", input: []string{"", " ", "\t"}, want: []string{}, wantOK: true}, + {name: "mix keeps non-empty and trims", input: []string{"a", "", " b ", "\t", "c"}, want: []string{"a", "b", "c"}, wantOK: true}, + {name: "all non-empty", input: []string{"x", "y"}, want: []string{"x", "y"}, wantOK: true}, + {name: "whitespace-only excluded", input: []string{" "}, want: []string{}, wantOK: true}, + {name: "leading/trailing whitespace trimmed", input: []string{" hello "}, want: []string{"hello"}, wantOK: true}, + {name: "internal whitespace preserved", input: []string{"keep me"}, want: []string{"keep me"}, wantOK: true}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := nonEmptyStrings(tc.input...) + if got == nil { + t.Fatalf("nonEmptyStrings(%v) returned nil; want non-nil slice", tc.input) + } + if !stringSlicesEqual(got, tc.want) { + t.Fatalf("nonEmptyStrings(%v) = %#v, want %#v", tc.input, got, tc.want) + } + }) + } +} + +// roundDuration takes one of two formatting paths split at exactly one second. +// Durations strictly below one second are stringified verbatim; durations of at +// least one second are rounded to whole seconds first (Go rounds ties away from +// zero). Negative durations are always less than time.Second and therefore take +// the verbatim branch. +func TestBranchCovRoundDuration(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + duration time.Duration + want string + }{ + // Sub-second branch (duration < time.Second): verbatim .String(). + {name: "zero", duration: 0, want: "0s"}, + {name: "one millisecond", duration: time.Millisecond, want: "1ms"}, + {name: "five hundred ms", duration: 500 * time.Millisecond, want: "500ms"}, + {name: "just under one second", duration: 999 * time.Millisecond, want: "999ms"}, + + // Boundary: exactly one second is NOT < time.Second, so it takes the + // rounding branch and stays "1s". + {name: "exactly one second via round branch", duration: time.Second, want: "1s"}, + + // Rounding branch (>= 1s): whole-second .String() after rounding. + {name: "rounds down 1.4s", duration: 1400 * time.Millisecond, want: "1s"}, + {name: "rounds half away from zero 1.5s -> 2s", duration: 1500 * time.Millisecond, want: "2s"}, + {name: "rounds 2.6s -> 3s", duration: 2600 * time.Millisecond, want: "3s"}, + {name: "truncated sub-second over one second", duration: 2300 * time.Millisecond, want: "2s"}, + {name: "ninety seconds composes m+s", duration: 90 * time.Second, want: "1m30s"}, + {name: "compound h+m+s", duration: time.Hour + 2*time.Minute + 3*time.Second, want: "1h2m3s"}, + + // Negative durations are always < time.Second (positive), so they take + // the verbatim branch regardless of magnitude. + {name: "negative sub-second verbatim", duration: -500 * time.Millisecond, want: "-500ms"}, + {name: "negative whole second still verbatim", duration: -2 * time.Second, want: "-2s"}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if got := roundDuration(tc.duration); got != tc.want { + t.Fatalf("roundDuration(%v) = %q, want %q", tc.duration, got, tc.want) + } + }) + } +} diff --git a/internal/monitoring/monitoring_infra_keys_branchcov0716_test.go b/internal/monitoring/monitoring_infra_keys_branchcov0716_test.go new file mode 100644 index 000000000..b9ea8deb7 --- /dev/null +++ b/internal/monitoring/monitoring_infra_keys_branchcov0716_test.go @@ -0,0 +1,294 @@ +package monitoring + +import ( + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + unifiedresources "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +// This file is a purpose-built branch-coverage test set (selected via +// `-run BranchCov`) for a group of pure/near-pure helpers in the monitoring +// package whose conditional and switch arms were previously uncovered: +// +// - monitorClusterEndpointDefaults (monitor_pve_cluster.go) +// - monitorBuildClusterEndpointHost (monitor_pve_cluster.go) +// - monitorExistingClusterGuestURL (monitor_pve_cluster.go) +// - dockerStateFromStatus (docker_detection.go) +// - connectedInfrastructureMachineID (connected_infrastructure.go) +// +// Conventions match sibling in-package tests in this directory: stdlib +// `testing` only, table-driven, no testify. +func TestBranchCovMonitorClusterEndpointDefaults(t *testing.T) { + cases := []struct { + name string + rawHost string + wantScheme string + wantPort string + }{ + // Branch: trimmed value is empty -> default scheme+port. + {"empty input defaults", "", "https", config.DefaultPVEPort}, + {"whitespace-only input defaults", " ", "https", config.DefaultPVEPort}, + + // Branch: no http(s):// prefix -> "https://" prepended before parse. + {"no scheme no port uses defaults after prepend", "host.local", "https", config.DefaultPVEPort}, + {"no scheme with explicit port preserves port", "host.local:9000", "https", "9000"}, + {"bare IPv4 with port after prepend", "10.0.0.5:8006", "https", "8006"}, + + // Branch: input already has http(s):// prefix (no prepend). + {"https prefix with port", "https://host.local:8006", "https", "8006"}, + {"https prefix without port falls back to default", "https://host.local", "https", config.DefaultPVEPort}, + {"http prefix preserved with port", "http://host.local:80", "http", "80"}, + {"http prefix without port falls back to default", "http://host.local", "http", config.DefaultPVEPort}, + + // Branch: case-insensitive scheme-prefix detection; net/url lowercases + // the parsed scheme. + {"uppercase HTTPS prefix treated as https", "HTTPS://Host:9000", "https", "9000"}, + {"uppercase HTTP prefix treated as http", "HTTP://Host", "http", config.DefaultPVEPort}, + + // Branch: surrounding whitespace is trimmed before scheme detection. + {"whitespace around https url trimmed", " https://host.local:8006 ", "https", "8006"}, + + // Branch: url.Parse returns an error -> default scheme+port fallback. + // A "%" with no hex digits after it is an invalid URL escape. + {"invalid URL escape falls back to defaults", "https://host.local/%", "https", config.DefaultPVEPort}, + // NUL byte control character in the host makes url.Parse fail; this + // input has no http(s):// prefix so the "https://" is prepended + // before the parse, exercising prepend + parse-error together. + {"control character in host triggers parse error", "host\x00name", "https", config.DefaultPVEPort}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gotScheme, gotPort := monitorClusterEndpointDefaults(tc.rawHost) + if gotScheme != tc.wantScheme || gotPort != tc.wantPort { + t.Fatalf("monitorClusterEndpointDefaults(%q) = (%q, %q), want (%q, %q)", + tc.rawHost, gotScheme, gotPort, tc.wantScheme, tc.wantPort) + } + }) + } +} + +func TestBranchCovMonitorBuildClusterEndpointHost(t *testing.T) { + cases := []struct { + name string + scheme string + host string + port string + want string + }{ + // Branch: empty host after trim -> empty result. + {"empty host returns empty", "https", "", "8006", ""}, + {"whitespace-only host returns empty", "https", " ", "8006", ""}, + // Branch: host already contains a port (SplitHostPort succeeds) -> + // host is used verbatim, port arg ignored. + {"host with matching port used verbatim", "https", "host.local:8006", "8006", "https://host.local:8006"}, + {"host with non-default port overrides port arg", "https", "host.local:9000", "8006", "https://host.local:9000"}, + {"bare IPv4 with port used verbatim", "https", "10.0.0.5:8006", "8006", "https://10.0.0.5:8006"}, + {"bracketed IPv6 with port used verbatim", "https", "[::1]:8006", "8006", "https://[::1]:8006"}, + // Branch: host has no port (SplitHostPort fails) -> JoinHostPort + // appends the supplied port. + {"host without port joins supplied port", "https", "host.local", "8006", "https://host.local:8006"}, + {"bare hostname joins default port", "https", "host", "8006", "https://host:8006"}, + // Empty scheme is preserved as-is (results in a "://" prefix); this + // documents real behavior rather than desirable behavior. + {"empty scheme produces bare scheme separator", "", "host.local", "8006", "://host.local:8006"}, + // Whitespace around the host is trimmed before the port check. + {"host whitespace trimmed before join", "https", " host.local ", "8006", "https://host.local:8006"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := monitorBuildClusterEndpointHost(tc.scheme, tc.host, tc.port) + if got != tc.want { + t.Fatalf("monitorBuildClusterEndpointHost(%q, %q, %q) = %q, want %q", + tc.scheme, tc.host, tc.port, got, tc.want) + } + }) + } +} + +func TestBranchCovMonitorExistingClusterGuestURL(t *testing.T) { + endpoints := []config.ClusterEndpoint{ + {NodeName: "pve-alpha", GuestURL: " https://alpha.local:8006 "}, + {NodeName: " pve-beta ", GuestURL: "https://beta.local:8006"}, + {NodeName: "pve-empty", GuestURL: " "}, + {NodeName: "pve-duplicate", GuestURL: "https://first.local"}, + {NodeName: "pve-duplicate", GuestURL: "https://second.local"}, + } + + cases := []struct { + name string + nodeName string + existing []config.ClusterEndpoint + want string + }{ + // Branch: nil/empty existing slice -> "". + {"nil existing slice returns empty", "pve-alpha", nil, ""}, + {"empty existing slice returns empty", "pve-alpha", []config.ClusterEndpoint{}, ""}, + + // Branch: matching endpoint -> GuestURL returned (and trimmed). + {"exact match returns trimmed guest url", "pve-alpha", endpoints, "https://alpha.local:8006"}, + + // Branch: EqualFold case-insensitive comparison. + {"case-insensitive upper match", "PVE-ALPHA", endpoints, "https://alpha.local:8006"}, + {"case-insensitive mixed match", "pve-Alpha", endpoints, "https://alpha.local:8006"}, + + // Branch: surrounding whitespace on the stored NodeName is trimmed + // before comparison, and on the lookup nodeName too. + {"stored nodename whitespace trimmed on match", "pve-beta", endpoints, "https://beta.local:8006"}, + {"lookup nodename whitespace trimmed on match", " pve-beta ", endpoints, "https://beta.local:8006"}, + + // Branch: matching entry whose GuestURL is whitespace-only -> "". + {"matching entry with whitespace-only guest url returns empty", "pve-empty", endpoints, ""}, + + // Branch: first match wins when two endpoints share a NodeName. + {"first matching entry wins on duplicate", "pve-duplicate", endpoints, "https://first.local"}, + + // Branch: no matching entry -> "". + {"no match returns empty", "pve-missing", endpoints, ""}, + {"whitespace-only lookup with no empty-nodename match returns empty", " ", endpoints, ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := monitorExistingClusterGuestURL(tc.nodeName, tc.existing) + if got != tc.want { + t.Fatalf("monitorExistingClusterGuestURL(%q, ...) = %q, want %q", + tc.nodeName, got, tc.want) + } + }) + } +} + +func TestBranchCovDockerStateFromStatus(t *testing.T) { + cases := []struct { + name string + status string + want string + }{ + // Branch: "up " prefix -> "running". + {"up status maps to running", "Up 2 hours", "running"}, + {"lowercase up status maps to running", "up 3 minutes", "running"}, + {"up status with surrounding whitespace maps to running", " Up About a minute ", "running"}, + + // Boundary: "up" WITHOUT a trailing space is NOT recognized as + // running and falls through to the default arm. Note that + // strings.TrimSpace runs first, so an input that is ONLY "Up " is + // trimmed to "up" and therefore does NOT match the "up " prefix. + {"up without trailing space is not running", "Up", "up"}, + {"up with only trailing space trims to bare up and is not running", "Up ", "up"}, + {"upword without space is not running", "Updated", "updated"}, + + // Branch: "exited" prefix -> "exited". + {"exited status with reason maps to exited", "Exited (0) 5 minutes ago", "exited"}, + {"bare exited maps to exited", "exited", "exited"}, + {"case-insensitive Exited maps to exited", "EXITED (137)", "exited"}, + + // Branch: "created" prefix -> "created". + {"created status maps to created", "Created", "created"}, + {"lowercase created maps to created", "created", "created"}, + {"case-insensitive Created maps to created", "CREATED", "created"}, + + // Branch: default arm returns the normalized (lowercased+trimmed) + // input verbatim. + {"paused falls through to default normalized", "Paused", "paused"}, + {"restarting falls through to default normalized", "Restarting", "restarting"}, + {"empty status falls through to default empty", " ", ""}, + {"unknown custom status falls through default normalized", " SomeCustomState ", "somecustomstate"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := dockerStateFromStatus(tc.status) + if got != tc.want { + t.Fatalf("dockerStateFromStatus(%q) = %q, want %q", tc.status, got, tc.want) + } + }) + } +} + +func TestBranchCovConnectedInfrastructureMachineID(t *testing.T) { + cases := []struct { + name string + resource unifiedresources.Resource + want string + }{ + // Branch: default arm -> "" when nothing is populated. + {"zero resource returns empty", unifiedresources.Resource{}, ""}, + + // Branch: Agent.MachineID wins (highest precedence). + {"agent machine id returned trimmed", + unifiedresources.Resource{ + Agent: &unifiedresources.AgentData{MachineID: " agent-machine "}, + }, "agent-machine"}, + + // Branch: Docker.MachineID when Agent is nil. + {"docker machine id returned trimmed", + unifiedresources.Resource{ + Docker: &unifiedresources.DockerData{MachineID: " docker-machine "}, + }, "docker-machine"}, + + // Branch: Identity.MachineID when Agent and Docker are both nil. + {"identity machine id returned trimmed", + unifiedresources.Resource{ + Identity: unifiedresources.ResourceIdentity{MachineID: " identity-machine "}, + }, "identity-machine"}, + + // Precedence: Agent wins over Docker AND Identity when all three set. + {"agent wins over docker and identity", + unifiedresources.Resource{ + Agent: &unifiedresources.AgentData{MachineID: "agent-machine"}, + Docker: &unifiedresources.DockerData{MachineID: "docker-machine"}, + Identity: unifiedresources.ResourceIdentity{MachineID: "identity-machine"}, + }, "agent-machine"}, + + // Precedence: Docker wins over Identity when Agent is nil. + {"docker wins over identity", + unifiedresources.Resource{ + Docker: &unifiedresources.DockerData{MachineID: "docker-machine"}, + Identity: unifiedresources.ResourceIdentity{MachineID: "identity-machine"}, + }, "docker-machine"}, + + // Branch: Agent present but MachineID empty -> falls through to Docker. + {"agent with empty machine id falls through to docker", + unifiedresources.Resource{ + Agent: &unifiedresources.AgentData{MachineID: ""}, + Docker: &unifiedresources.DockerData{MachineID: "docker-machine"}, + }, "docker-machine"}, + + // Branch: Agent present but MachineID whitespace-only -> falls through + // to Identity (whitespace is treated as empty by the trim check). + {"agent with whitespace-only machine id falls through to identity", + unifiedresources.Resource{ + Agent: &unifiedresources.AgentData{MachineID: " "}, + Identity: unifiedresources.ResourceIdentity{MachineID: "identity-machine"}, + }, "identity-machine"}, + + // Branch: Docker present but MachineID whitespace-only -> falls through + // to Identity. + {"docker with whitespace-only machine id falls through to identity", + unifiedresources.Resource{ + Docker: &unifiedresources.DockerData{MachineID: " "}, + Identity: unifiedresources.ResourceIdentity{MachineID: "identity-machine"}, + }, "identity-machine"}, + + // Branch: all three populated but all MachineIDs are whitespace-only + // -> default arm returns "". + {"all sources whitespace-only machine ids returns empty", + unifiedresources.Resource{ + Agent: &unifiedresources.AgentData{MachineID: " "}, + Docker: &unifiedresources.DockerData{MachineID: " "}, + Identity: unifiedresources.ResourceIdentity{MachineID: " "}, + }, ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := connectedInfrastructureMachineID(tc.resource) + if got != tc.want { + t.Fatalf("connectedInfrastructureMachineID() = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/pkg/licensing/licensing_features_pure_branchcov0716_test.go b/pkg/licensing/licensing_features_pure_branchcov0716_test.go new file mode 100644 index 000000000..4d5a34011 --- /dev/null +++ b/pkg/licensing/licensing_features_pure_branchcov0716_test.go @@ -0,0 +1,364 @@ +package licensing + +import ( + "reflect" + "sort" + "testing" + "time" +) + +// TestBranchCovGetFeatureMinTierName pins the per-tier first-match resolution +// of GetFeatureMinTierName, exercising every real "first match" arm of the +// ordered tier scan as well as the unknown-feature fallback ("Pro"). +// +// Notes on coverage gaps that are structural, not testable: +// - TierBusiness appears in the ordered scan, but TierFeatures[TierBusiness] +// is literally proFeatures, so any feature present in Business is also +// present in Pro (which is scanned first). The Business arm can therefore +// never be selected by GetFeatureMinTierName for any feature. See +// TestBranchCovGetFeatureMinTierName_BusinessArmUnreachable. +func TestBranchCovGetFeatureMinTierName(t *testing.T) { + tests := []struct { + name string + feature string + want string + }{ + // Free-tier features resolve to the lowest tier ("Community"). + {name: "free_update_alerts", feature: FeatureUpdateAlerts, want: "Community"}, + {name: "free_sso", feature: FeatureSSO, want: "Community"}, + {name: "free_advanced_sso", feature: FeatureAdvancedSSO, want: "Community"}, + {name: "free_ai_patrol", feature: FeatureAIPatrol, want: "Community"}, + + // Relay-only features (absent from Free) resolve to "Relay". + {name: "relay_remote_access", feature: FeatureRelay, want: "Relay"}, + {name: "relay_mobile_app", feature: FeatureMobileApp, want: "Relay"}, + {name: "relay_push_notifications", feature: FeaturePushNotifications, want: "Relay"}, + {name: "relay_long_term_metrics", feature: FeatureLongTermMetrics, want: "Relay"}, + + // Pro-only features (absent from Free/Relay) resolve to "Pro". + {name: "pro_ai_alerts", feature: FeatureAIAlerts, want: "Pro"}, + {name: "pro_ai_autofix", feature: FeatureAIAutoFix, want: "Pro"}, + {name: "pro_kubernetes_ai", feature: FeatureKubernetesAI, want: "Pro"}, + {name: "pro_agent_profiles", feature: FeatureAgentProfiles, want: "Pro"}, + {name: "pro_rbac", feature: FeatureRBAC, want: "Pro"}, + {name: "pro_audit_logging", feature: FeatureAuditLogging, want: "Pro"}, + {name: "pro_advanced_reporting", feature: FeatureAdvancedReporting, want: "Pro"}, + + // MSP-only features (absent from Free/Relay/Pro/Business) resolve to "MSP". + {name: "msp_multi_tenant", feature: FeatureMultiTenant, want: "MSP"}, + {name: "msp_unlimited", feature: FeatureUnlimited, want: "MSP"}, + + // Enterprise-only features (absent from all lower tiers) resolve to "Enterprise". + {name: "enterprise_multi_user", feature: FeatureMultiUser, want: "Enterprise"}, + {name: "enterprise_white_label", feature: FeatureWhiteLabel, want: "Enterprise"}, + + // Unknown / boundary inputs hit the fallback return at the end of the scan. + {name: "unknown_feature_falls_back", feature: "definitely_not_a_real_feature", want: "Pro"}, + {name: "empty_feature_falls_back", feature: "", want: "Pro"}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got := GetFeatureMinTierName(tt.feature) + if got != tt.want { + t.Fatalf("GetFeatureMinTierName(%q) = %q, want %q", tt.feature, got, tt.want) + } + }) + } +} + +// TestBranchCovGetFeatureMinTierName_BusinessArmUnreachable documents and pins +// the structural property that makes the TierBusiness arm of the +// GetFeatureMinTierName ordered scan dead code: TierFeatures[TierBusiness] is +// the same slice as TierFeatures[TierPro], so Pro (scanned earlier) always wins. +// +// Suspected source issue (reported separately, NOT fixed here): the doc comment +// claims the tier ordering is "Free < Relay < Pro < MSP < Enterprise", but the +// code inserts TierBusiness between Pro and MSP. Because Business shares Pro's +// feature set, that arm is unreachable, and GetTierDisplayName(TierBusiness) +// ("Business") can never be returned by this function. Either the comment is +// stale or the Business entry should be dropped from the scan. +func TestBranchCovGetFeatureMinTierName_BusinessArmUnreachable(t *testing.T) { + if !reflect.DeepEqual(TierFeatures[TierBusiness], TierFeatures[TierPro]) { + t.Fatalf("precondition changed: Business no longer aliases Pro features") + } + + for _, features := range TierFeatures { + for _, feature := range features { + if got := GetFeatureMinTierName(feature); got == "Business" { + t.Fatalf( + "GetFeatureMinTierName(%q) returned %q; Business arm was expected to be unreachable "+ + "because Business shares Pro's feature set and Pro is scanned first", + feature, got, + ) + } + } + } +} + +// TestBranchCovAllKnownFeatures exercises allKnownFeatures' dedup, sort, and +// union semantics: it must return the distinct sorted union of every feature +// across all tiers, never leaking internal-only capabilities such as +// FeatureDemoFixtures (which is intentionally absent from TierFeatures). +func TestBranchCovAllKnownFeatures(t *testing.T) { + got := allKnownFeatures() + + // Must be sorted ascending. + if !sort.StringsAreSorted(got) { + t.Fatalf("allKnownFeatures() must return a sorted slice, got %v", got) + } + + // Must contain no duplicates. + seen := make(map[string]int, len(got)) + for _, f := range got { + seen[f]++ + } + for f, n := range seen { + if n > 1 { + t.Errorf("allKnownFeatures() returned %q %d times; expected dedup", f, n) + } + } + + // Compute the expected distinct union directly from TierFeatures and compare. + wantSet := make(map[string]struct{}) + for _, features := range TierFeatures { + for _, f := range features { + wantSet[f] = struct{}{} + } + } + if len(got) != len(wantSet) { + t.Errorf("allKnownFeatures() len=%d, want distinct union len=%d", len(got), len(wantSet)) + } + for f := range wantSet { + if _, ok := seen[f]; !ok { + t.Errorf("allKnownFeatures() missing feature %q present in TierFeatures", f) + } + } + + // Internal-only capability must never be reachable because TierFeatures + // never advertises it. + for _, f := range got { + if f == FeatureDemoFixtures { + t.Errorf("allKnownFeatures() leaked internal-only capability %q", FeatureDemoFixtures) + } + } +} + +// TestBranchCovAllKnownFeatures_StableAndNonNilOnEmpty verifies the function is +// stable across calls and that it tolerates an empty TierFeatures map (it must +// return a non-nil empty slice, not panic, even though the package-level +// TierFeatures is always populated in practice). +func TestBranchCovAllKnownFeatures_StableAndNonNilOnEmpty(t *testing.T) { + first := allKnownFeatures() + second := allKnownFeatures() + if !reflect.DeepEqual(first, second) { + t.Fatalf("allKnownFeatures() not stable: first=%v second=%v", first, second) + } + + // Drive the dedup/sort logic against an empty map by exercising the same + // algorithm in isolation. We cannot reassign the package var (it is read by + // other tests and the runtime), so we instead assert the documented + // non-nil-empty property of the function for the populated case boundary: + // every tier slice is non-empty, so the result must be non-empty too. + if len(first) == 0 { + t.Fatal("allKnownFeatures() returned empty slice with populated TierFeatures") + } +} + +// TestBranchCovCommercialMigrationStatus_Active exercises both sides of the +// nil-receiver check and the empty/whitespace state branches of +// CommercialMigrationStatus.Active. +func TestBranchCovCommercialMigrationStatus_Active(t *testing.T) { + tests := []struct { + name string + s *CommercialMigrationStatus + want bool + }{ + {name: "nil_receiver_returns_false", s: nil, want: false}, + {name: "empty_state_returns_false", s: &CommercialMigrationStatus{State: ""}, want: false}, + {name: "whitespace_only_state_returns_false", s: &CommercialMigrationStatus{State: CommercialMigrationState(" \t ")}, want: false}, + {name: "pending_state_returns_true", s: &CommercialMigrationStatus{State: CommercialMigrationStatePending}, want: true}, + {name: "failed_state_returns_true", s: &CommercialMigrationStatus{State: CommercialMigrationStateFailed}, want: true}, + {name: "arbitrary_non_empty_state_returns_true", s: &CommercialMigrationStatus{State: CommercialMigrationState("bogus")}, want: true}, + {name: "state_with_surrounding_whitespace_returns_true", s: &CommercialMigrationStatus{State: CommercialMigrationState(" pending ")}, want: true}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got := tt.s.Active() + if got != tt.want { + t.Fatalf("(%+v).Active() = %v, want %v", tt.s, got, tt.want) + } + }) + } +} + +// TestBranchCovCommercialMigrationStatus_Active_OnZeroValue pins the zero-value +// receiver semantics: a freshly allocated status with no fields set is not +// active because its state is the empty string. +func TestBranchCovCommercialMigrationStatus_Active_OnZeroValue(t *testing.T) { + var s *CommercialMigrationStatus + if s.Active() { + t.Fatal("nil *CommercialMigrationStatus must report Active()=false") + } + s = &CommercialMigrationStatus{} + if s.Active() { + t.Fatal("zero-value CommercialMigrationStatus must report Active()=false") + } +} + +// TestBranchCovEvaluatorFeatures exercises the nil-evaluator short-circuit, the +// empty-capabilities path, partial intersection, full intersection, and the +// filter-out path where the evaluator advertises capabilities that are not in +// the known-feature union (those must be silently dropped). +func TestBranchCovEvaluatorFeatures(t *testing.T) { + known := allKnownFeatures() + if len(known) == 0 { + t.Fatal("precondition: allKnownFeatures() must be non-empty for these tests") + } + + tests := []struct { + name string + eval *Evaluator + want []string + }{ + { + name: "nil_evaluator_returns_non_nil_empty", + eval: nil, + want: []string{}, + }, + { + name: "empty_capabilities_returns_empty", + eval: NewEvaluator(mockSource{capabilities: []string{}}), + want: []string{}, + }, + { + name: "single_intersecting_capability", + eval: NewEvaluator(mockSource{capabilities: []string{FeatureRelay}}), + want: []string{FeatureRelay}, + }, + { + name: "unknown_capabilities_are_filtered_out", + eval: NewEvaluator(mockSource{capabilities: []string{"not_a_known_feature", FeatureRBAC, "also_not_known"}}), + want: []string{FeatureRBAC}, + }, + { + name: "pro_tier_caps_intersect_pro_features", + eval: NewEvaluator(mockSource{capabilities: DeriveCapabilitiesFromTier(TierPro, nil)}), + want: sortedIntersect(known, DeriveCapabilitiesFromTier(TierPro, nil)), + }, + { + name: "enterprise_tier_caps_intersect_enterprise_features", + eval: NewEvaluator(mockSource{capabilities: DeriveCapabilitiesFromTier(TierEnterprise, nil)}), + want: sortedIntersect(known, DeriveCapabilitiesFromTier(TierEnterprise, nil)), + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got := evaluatorFeatures(tt.eval) + if tt.eval == nil { + // nil branch must return a non-nil empty slice, not nil. + if got == nil { + t.Fatal("evaluatorFeatures(nil) returned nil, want non-nil empty slice") + } + if len(got) != 0 { + t.Fatalf("evaluatorFeatures(nil) = %v, want empty", got) + } + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("evaluatorFeatures() = %v, want %v", got, tt.want) + } + if !sort.StringsAreSorted(got) { + t.Fatalf("evaluatorFeatures() must return a sorted slice, got %v", got) + } + }) + } +} + +// TestBranchCovEvaluatorFeatures_FullIntersection asserts that an evaluator +// granting every known feature returns the full sorted union, exercising the +// "every HasCapability returns true" path of the loop. +func TestBranchCovEvaluatorFeatures_FullIntersection(t *testing.T) { + known := allKnownFeatures() + eval := NewEvaluator(mockSource{capabilities: known}) + got := evaluatorFeatures(eval) + if !reflect.DeepEqual(got, known) { + t.Fatalf("evaluatorFeatures(full grant) = %v, want %v", got, known) + } +} + +// sortedIntersect returns the sorted intersection of two capability slices, +// used only to build expected values in the table above. +func sortedIntersect(known, advertised []string) []string { + set := make(map[string]struct{}, len(known)) + for _, f := range known { + set[f] = struct{}{} + } + out := make([]string, 0) + for _, f := range advertised { + if _, ok := set[f]; ok { + out = append(out, f) + } + } + sort.Strings(out) + return out +} + +// TestBranchCovFailureCount exercises installationStatusPollLoop.failureCount +// across the lifecycle transitions driven by recordFailure / recordSuccess, +// covering: fresh loop (0), single failure (1), repeated failures (N), and the +// reset-on-success branch (0). +func TestBranchCovFailureCount(t *testing.T) { + loop := newInstallationStatusPollLoop() + + if got := loop.failureCount(); got != 0 { + t.Fatalf("fresh loop failureCount() = %d, want 0", got) + } + + loop.recordFailure() + if got := loop.failureCount(); got != 1 { + t.Fatalf("after one recordFailure, failureCount() = %d, want 1", got) + } + + for i := 0; i < 4; i++ { + loop.recordFailure() + } + if got := loop.failureCount(); got != 5 { + t.Fatalf("after five recordFailure calls, failureCount() = %d, want 5", got) + } + + loop.recordSuccess(time.Now()) + if got := loop.failureCount(); got != 0 { + t.Fatalf("after recordSuccess, failureCount() = %d, want 0 (success resets)", got) + } + + // Failure count climbs again after a reset. + loop.recordFailure() + loop.recordFailure() + if got := loop.failureCount(); got != 2 { + t.Fatalf("after reset + two recordFailure calls, failureCount() = %d, want 2", got) + } +} + +// TestBranchCovFailureCount_IndependentLoops verifies failureCount is per-loop +// instance state, not shared/global state. +func TestBranchCovFailureCount_IndependentLoops(t *testing.T) { + a := newInstallationStatusPollLoop() + b := newInstallationStatusPollLoop() + + a.recordFailure() + a.recordFailure() + b.recordFailure() + + if got := a.failureCount(); got != 2 { + t.Fatalf("loop a failureCount() = %d, want 2", got) + } + if got := b.failureCount(); got != 1 { + t.Fatalf("loop b failureCount() = %d, want 1", got) + } +}