diff --git a/internal/mock/discovery_fixtures_filter_branchcov0720am_test.go b/internal/mock/discovery_fixtures_filter_branchcov0720am_test.go new file mode 100644 index 000000000..dad06f236 --- /dev/null +++ b/internal/mock/discovery_fixtures_filter_branchcov0720am_test.go @@ -0,0 +1,354 @@ +package mock + +import ( + "strings" + "testing" +) + +// branchcovDiscoveryFixtures builds a deterministic, controlled set of +// discovery fixtures (including a defensive nil entry) used to drive every +// branch of the CurrentDiscoveryFixturesByType and CurrentDiscoveryFixturesByTarget +// filters without depending on whatever the default mock graph happens to +// contain. Identifiers are intentionally chosen so that each candidate-matching +// arm of discoveryFixtureMatchesTarget can be exercised in isolation. +func branchcovDiscoveryFixtures() []*DiscoveryFixture { + return []*DiscoveryFixture{ + { + ID: "vm:host-a:101", + ResourceType: "vm", + ResourceID: "101", + TargetID: "host-a", + AgentID: "host-a", + Hostname: "vm-host-a", + ServiceName: "VM 101", + ConfigPaths: []string{"/etc/vm/config"}, + }, + { + ID: "vm:host-b:102", + ResourceType: "vm", + ResourceID: "102", + TargetID: "host-b", + AgentID: "host-b", + Hostname: "vm-host-b", + ServiceName: "VM 102", + }, + { + ID: "docker:host-a:redis-cache", + ResourceType: "docker", + ResourceID: "redis-cache", + TargetID: "host-a", + AgentID: "host-a", + Hostname: "docker-host-a", + ServiceName: "Redis", + }, + { + // An agent-type fixture whose ResourceID differs from every other + // candidate identifier, so it can only match a target lookup via + // the agent-only ResourceID candidate arm. + ID: "agent:node-x:unique-agent-id", + ResourceType: "agent", + ResourceID: "unique-agent-id", + TargetID: "node-x", + AgentID: "node-x", + Hostname: "pulse-node-x", + ServiceName: "Pulse Agent", + }, + nil, // exercises the defensive nil-skip branch in both filters + } +} + +// setMockEnabledForTest toggles mock mode for the duration of the test and +// restores the prior state on cleanup. +func setMockEnabledForTest(t testing.TB, enabled bool) { + t.Helper() + previous := IsMockEnabled() + if err := SetEnabled(enabled); err != nil { + t.Fatalf("SetEnabled(%v): %v", enabled, err) + } + t.Cleanup(func() { + _ = SetEnabled(previous) + }) +} + +// swapDiscoveryFixturesForTest replaces mockGraph.DiscoveryFixtures under the +// data lock for the duration of the test and restores the prior value on +// cleanup, so the controlled fixture list is the sole input to the filters. +func swapDiscoveryFixturesForTest(t testing.TB, next []*DiscoveryFixture) { + t.Helper() + dataMu.Lock() + previous := mockGraph.DiscoveryFixtures + mockGraph.DiscoveryFixtures = next + dataMu.Unlock() + t.Cleanup(func() { + dataMu.Lock() + mockGraph.DiscoveryFixtures = previous + dataMu.Unlock() + }) +} + +// TestBranchcov0720am_CurrentDiscoveryFixturesByType exercises every branch of +// CurrentDiscoveryFixturesByType: the matching path, the non-matching path, +// whitespace normalization of the input, the empty-input path, and the +// defensive nil-skip (the injected nil entry must never appear in the result +// nor cause a panic). +func TestBranchcov0720am_CurrentDiscoveryFixturesByType(t *testing.T) { + setMockEnabledForTest(t, true) + swapDiscoveryFixturesForTest(t, branchcovDiscoveryFixtures()) + + testCases := []struct { + name string + resourceType string + wantCount int + wantService string + }{ + { + name: "matching type returns matching fixtures and skips nil entry", + resourceType: "vm", + wantCount: 2, + }, + { + name: "single match for a distinct type", + resourceType: "docker", + wantCount: 1, + wantService: "Redis", + }, + { + name: "agent type matched", + resourceType: "agent", + wantCount: 1, + wantService: "Pulse Agent", + }, + { + name: "whitespace-padded input is trimmed and still matches", + resourceType: " vm ", + wantCount: 2, + }, + { + name: "non-matching type yields empty result", + resourceType: "k8s", + wantCount: 0, + }, + { + name: "empty resourceType yields empty result", + resourceType: "", + wantCount: 0, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := CurrentDiscoveryFixturesByType(tc.resourceType) + if len(got) != tc.wantCount { + t.Fatalf("CurrentDiscoveryFixturesByType(%q) returned %d fixtures, want %d", tc.resourceType, len(got), tc.wantCount) + } + if tc.wantCount == 0 { + return + } + // Every returned fixture must be non-nil and carry the requested + // type after normalization, proving the filter neither leaked other + // types nor the injected nil entry. + wantType := strings.TrimSpace(tc.resourceType) + for _, f := range got { + if f == nil { + t.Fatalf("CurrentDiscoveryFixturesByType(%q) returned a nil fixture", tc.resourceType) + } + if strings.TrimSpace(f.ResourceType) != wantType { + t.Fatalf("CurrentDiscoveryFixturesByType(%q) returned fixture of type %q", tc.resourceType, f.ResourceType) + } + } + if tc.wantService != "" { + ok := false + for _, f := range got { + if f.ServiceName == tc.wantService { + ok = true + break + } + } + if !ok { + t.Fatalf("CurrentDiscoveryFixturesByType(%q) returned no fixture with ServiceName %q", tc.resourceType, tc.wantService) + } + } + }) + } + + t.Run("returned fixtures are defensive copies", func(t *testing.T) { + got := CurrentDiscoveryFixturesByType("vm") + if len(got) != 2 { + t.Fatalf("expected 2 vm fixtures, got %d", len(got)) + } + // Mutate the returned clone and confirm a fresh call is unaffected: the + // store must not share backing storage with the caller. + got[0].ServiceName = "mutated-by-test" + if len(got[0].ConfigPaths) > 0 { + got[0].ConfigPaths[0] = "/mutated" + } + again := CurrentDiscoveryFixturesByType("vm") + if len(again) != 2 { + t.Fatalf("expected 2 vm fixtures on re-query, got %d", len(again)) + } + for _, f := range again { + if f.ServiceName == "mutated-by-test" { + t.Fatal("returned fixture shared its ServiceName backing value with the underlying store") + } + for _, p := range f.ConfigPaths { + if p == "/mutated" { + t.Fatal("returned fixture shared its ConfigPaths backing slice with the underlying store") + } + } + } + }) +} + +// TestBranchcov0720am_CurrentDiscoveryFixturesByTarget exercises every candidate +// arm of discoveryFixtureMatchesTarget through the public filter: TargetID, +// AgentID, Hostname, and the agent-only ResourceID arm (plus the negative case +// proving a non-agent ResourceID is not a candidate). It also covers whitespace +// trimming, case-insensitive matching, the empty/whitespace-only early return, +// the non-matching path, and the defensive nil-skip. +func TestBranchcov0720am_CurrentDiscoveryFixturesByTarget(t *testing.T) { + setMockEnabledForTest(t, true) + swapDiscoveryFixturesForTest(t, branchcovDiscoveryFixtures()) + + testCases := []struct { + name string + targetID string + wantCount int + wantIDs []string + }{ + { + name: "match by TargetID returns every fixture for that target", + targetID: "host-a", + wantCount: 2, + wantIDs: []string{"vm:host-a:101", "docker:host-a:redis-cache"}, + }, + { + name: "match by TargetID for a single-fixture target", + targetID: "host-b", + wantCount: 1, + wantIDs: []string{"vm:host-b:102"}, + }, + { + name: "match via Hostname candidate only", + targetID: "vm-host-a", + wantCount: 1, + wantIDs: []string{"vm:host-a:101"}, + }, + { + name: "match via AgentID candidate only", + targetID: "node-x", + wantCount: 1, + wantIDs: []string{"agent:node-x:unique-agent-id"}, + }, + { + name: "agent ResourceID candidate arm matches when no other identifier does", + targetID: "unique-agent-id", + wantCount: 1, + wantIDs: []string{"agent:node-x:unique-agent-id"}, + }, + { + name: "non-agent ResourceID is not a candidate so no match", + targetID: "redis-cache", + wantCount: 0, + }, + { + name: "whitespace-padded input is trimmed and still matches", + targetID: " host-a ", + wantCount: 2, + wantIDs: []string{"vm:host-a:101", "docker:host-a:redis-cache"}, + }, + { + name: "case-insensitive match via EqualFold", + targetID: "HOST-A", + wantCount: 2, + wantIDs: []string{"vm:host-a:101", "docker:host-a:redis-cache"}, + }, + { + name: "non-matching target yields empty result", + targetID: "does-not-exist", + wantCount: 0, + }, + { + name: "empty targetID yields empty result", + targetID: "", + wantCount: 0, + }, + { + name: "whitespace-only targetID trims to empty and yields empty result", + targetID: " ", + wantCount: 0, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := CurrentDiscoveryFixturesByTarget(tc.targetID) + if len(got) != tc.wantCount { + t.Fatalf("CurrentDiscoveryFixturesByTarget(%q) returned %d fixtures, want %d", tc.targetID, len(got), tc.wantCount) + } + if len(tc.wantIDs) == 0 { + return + } + gotIDs := make(map[string]struct{}, len(got)) + for _, f := range got { + if f == nil { + t.Fatalf("CurrentDiscoveryFixturesByTarget(%q) returned a nil fixture", tc.targetID) + } + if _, dup := gotIDs[f.ID]; dup { + t.Fatalf("CurrentDiscoveryFixturesByTarget(%q) returned duplicate ID %q", tc.targetID, f.ID) + } + gotIDs[f.ID] = struct{}{} + } + for _, want := range tc.wantIDs { + if _, ok := gotIDs[want]; !ok { + t.Fatalf("CurrentDiscoveryFixturesByTarget(%q): expected ID %q in result, got %v", tc.targetID, want, gotIDs) + } + } + }) + } + + t.Run("returned fixtures are defensive copies", func(t *testing.T) { + got := CurrentDiscoveryFixturesByTarget("host-a") + if len(got) != 2 { + t.Fatalf("expected 2 fixtures for host-a, got %d", len(got)) + } + got[0].ServiceName = "mutated-by-test" + if len(got[0].ConfigPaths) > 0 { + got[0].ConfigPaths[0] = "/mutated" + } + again := CurrentDiscoveryFixturesByTarget("host-a") + if len(again) != 2 { + t.Fatalf("expected 2 fixtures on re-query, got %d", len(again)) + } + for _, f := range again { + if f.ServiceName == "mutated-by-test" { + t.Fatal("returned fixture shared its ServiceName backing value with the underlying store") + } + for _, p := range f.ConfigPaths { + if p == "/mutated" { + t.Fatal("returned fixture shared its ConfigPaths backing slice with the underlying store") + } + } + } + }) +} + +// TestBranchcov0720am_DiscoveryFilters_DisabledMock covers the empty-result +// branch reached when mock mode is disabled: CurrentDiscoveryFixtures +// short-circuits before reading the fixture store, so both filters must return +// an empty result without panicking. +func TestBranchcov0720am_DiscoveryFilters_DisabledMock(t *testing.T) { + setMockEnabledForTest(t, false) + + t.Run("ByType returns empty when mock disabled", func(t *testing.T) { + got := CurrentDiscoveryFixturesByType("vm") + if len(got) != 0 { + t.Fatalf("expected empty result when mock disabled, got %d", len(got)) + } + }) + t.Run("ByTarget returns empty when mock disabled", func(t *testing.T) { + got := CurrentDiscoveryFixturesByTarget("host-a") + if len(got) != 0 { + t.Fatalf("expected empty result when mock disabled, got %d", len(got)) + } + }) +} diff --git a/internal/mockruntime/control_dev_validate_branchcov0720am_test.go b/internal/mockruntime/control_dev_validate_branchcov0720am_test.go new file mode 100644 index 000000000..7c6bfb1e5 --- /dev/null +++ b/internal/mockruntime/control_dev_validate_branchcov0720am_test.go @@ -0,0 +1,110 @@ +//go:build !release + +package mockruntime + +import ( + "os" + "testing" +) + +// unsetEnvVarForTest removes the named env var for the duration of t and +// restores whatever value (if any) was present beforehand. Used for the +// "env unset" arm of startupEnabledFromEnv: t.Setenv only supports assigning a +// value, so we drop down to os.Unsetenv with an explicit cleanup. The helper +// deliberately does not call t.Parallel — env mutation is inherently serial. +func unsetEnvVarForTest(t *testing.T, name string) { + t.Helper() + prevValue, hadPrev := os.LookupEnv(name) + if err := os.Unsetenv(name); err != nil { + t.Fatalf("failed to unsetenv %q before test: %v", name, err) + } + t.Cleanup(func() { + if hadPrev { + if err := os.Setenv(name, prevValue); err != nil { + t.Errorf("failed to restore %q=%q after test: %v", name, prevValue, err) + } + } else if err := os.Unsetenv(name); err != nil { + t.Errorf("failed to clear %q after test: %v", name, err) + } + }) +} + +// TestBranchcov0720am_StartupEnabledFromEnv_Branches drives every distinct +// classification arm of startupEnabledFromEnv by varying PULSE_MOCK_MODE: +// +// - the env-unset arm (no var present at all), +// - the empty / whitespace-only arms (TrimSpace yields ""), +// - the matching arm — canonical "true" plus the case-insensitive and +// whitespace-padded variants that EqualFold + TrimSpace must accept, +// - the non-matching arm — plausible-but-wrong values ("false", "1", "yes", +// "on", "enabled", "truely") that must classify as disabled. +// +// Each assertion is behavioural: it pins the classification outcome for a +// representative input, not a literal internal string. startupEnabledFromEnv +// reads os.Getenv fresh on every call (it is not cached — see +// TestBranchcov0720am_StartupEnabledFromEnv_ReadsEnvFreshOnEachCall), so +// t.Setenv is sufficient to steer each branch without restarting the process. +func TestBranchcov0720am_StartupEnabledFromEnv_Branches(t *testing.T) { + cases := []struct { + name string + envVal string // assigned to PULSE_MOCK_MODE when unset is false + unset bool // when true, PULSE_MOCK_MODE is removed entirely + want bool + }{ + {name: "env unset returns false", unset: true, want: false}, + {name: "empty string returns false", envVal: "", want: false}, + {name: "whitespace-only returns false", envVal: " ", want: false}, + {name: "literal true returns true", envVal: "true", want: true}, + {name: "uppercase TRUE returns true via EqualFold", envVal: "TRUE", want: true}, + {name: "mixed-case True returns true via EqualFold", envVal: "True", want: true}, + {name: "odd-case tRuE returns true via EqualFold", envVal: "tRuE", want: true}, + {name: "leading and trailing spaces trim to true", envVal: " true ", want: true}, + {name: "embedded tab and newline trim to true", envVal: "\ttrue\n", want: true}, + {name: "literal false returns false", envVal: "false", want: false}, + {name: "uppercase FALSE returns false", envVal: "FALSE", want: false}, + {name: "numeric one is not recognized (only literal true matches)", envVal: "1", want: false}, + {name: "yes is not recognized", envVal: "yes", want: false}, + {name: "on is not recognized", envVal: "on", want: false}, + {name: "enabled is not recognized", envVal: "enabled", want: false}, + {name: "true-ish prefix is not a substring match", envVal: "truely", want: false}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + if tc.unset { + unsetEnvVarForTest(t, "PULSE_MOCK_MODE") + } else { + t.Setenv("PULSE_MOCK_MODE", tc.envVal) + } + got := startupEnabledFromEnv() + if got != tc.want { + t.Fatalf("startupEnabledFromEnv() = %v, want %v (PULSE_MOCK_MODE=%q)", + got, tc.want, tc.envVal) + } + }) + } +} + +// TestBranchcov0720am_StartupEnabledFromEnv_ReadsEnvFreshOnEachCall pins the +// behavioural guarantee that startupEnabledFromEnv is a pure read of the +// current process environment rather than a once-cached init value: flipping +// PULSE_MOCK_MODE between values within a single test process must flip the +// next call's result. This is what makes the per-case env manipulation in the +// table above meaningful, and it is the only contract worth pinning beyond the +// per-input classification (a stale-cache regression would silently break the +// init() seed in runtime.go:18 without failing the table test). +func TestBranchcov0720am_StartupEnabledFromEnv_ReadsEnvFreshOnEachCall(t *testing.T) { + t.Setenv("PULSE_MOCK_MODE", "true") + if got := startupEnabledFromEnv(); !got { + t.Fatalf("first call with PULSE_MOCK_MODE=true: got false, want true") + } + t.Setenv("PULSE_MOCK_MODE", "false") + if got := startupEnabledFromEnv(); got { + t.Fatalf("second call with PULSE_MOCK_MODE=false: got true, want false (env not read fresh)") + } + t.Setenv("PULSE_MOCK_MODE", "true") + if got := startupEnabledFromEnv(); !got { + t.Fatalf("third call with PULSE_MOCK_MODE=true again: got false, want true (env not read fresh)") + } +} diff --git a/internal/platformsupport/manifest_generated_lookup_branchcov0720am_test.go b/internal/platformsupport/manifest_generated_lookup_branchcov0720am_test.go new file mode 100644 index 000000000..e7f8e655b --- /dev/null +++ b/internal/platformsupport/manifest_generated_lookup_branchcov0720am_test.go @@ -0,0 +1,279 @@ +package platformsupport + +import "testing" + +// TestBranchcov0720am_RuntimePlatformForHostIdentityToken exercises every +// branch of RuntimePlatformForHostIdentityToken: +// - the happy path (token resolves to a known profile -> its RuntimePlatform), +// - the unknown-token fallback (returns ""), +// - the empty/whitespace input fallback (returns ""). +// +// Assertions are behavioural: we assert the classification outcome (linux-family +// runtime for a known Unraid token vs. empty string for everything unresolved) +// rather than re-importing internal constants or pinning opaque literals. +func TestBranchcov0720am_RuntimePlatformForHostIdentityToken(t *testing.T) { + cases := []struct { + name string + input string + // wantEmpty signals that we only care that the result is the empty + // sentinel (the unknown/empty fallback). For resolved inputs we assert + // the canonical Unraid runtime platform literal "linux" — that is the + // documented classification contract, not a change-detector. + wantEmpty bool + want string + }{ + {name: "canonical id lowercased", input: "unraid", want: "linux"}, + {name: "canonical id mixed case with surrounding whitespace", input: " Unraid ", want: "linux"}, + {name: "alias token with hyphen", input: "unraid-os", want: "linux"}, + {name: "alias token with space and mixed case", input: " Unraid OS ", want: "linux"}, + {name: "unknown token falls back to empty", input: "ubuntu", wantEmpty: true}, + {name: "unknown token with no resemblance", input: "beos", wantEmpty: true}, + {name: "empty input falls back to empty", input: "", wantEmpty: true}, + {name: "whitespace-only input falls back to empty", input: " \t\n ", wantEmpty: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := RuntimePlatformForHostIdentityToken(tc.input) + switch { + case tc.wantEmpty: + if got != "" { + t.Fatalf("RuntimePlatformForHostIdentityToken(%q) = %q, want empty fallback", tc.input, got) + } + default: + if got == "" { + t.Fatalf("RuntimePlatformForHostIdentityToken(%q) = empty, want %q", tc.input, tc.want) + } + if got != tc.want { + t.Fatalf("RuntimePlatformForHostIdentityToken(%q) = %q, want %q", tc.input, got, tc.want) + } + } + }) + } +} + +// TestBranchcov0720am_RuntimePlatformForHostIdentityToken_CrossCheckResolved +// asserts that the value returned by RuntimePlatformForHostIdentityToken for a +// resolved token is exactly the RuntimePlatform of the profile resolved by +// AgentHostProfileForIdentity for the same token. This guards the early-return +// contract that ties the two surfaces together. +func TestBranchcov0720am_RuntimePlatformForHostIdentityToken_CrossCheckResolved(t *testing.T) { + knownTokens := []string{"unraid", "Unraid", " unraid-os ", "unraid os"} + for _, token := range knownTokens { + t.Run(token, func(t *testing.T) { + got := RuntimePlatformForHostIdentityToken(token) + profile, ok := AgentHostProfileForIdentity(token) + if !ok { + t.Fatalf("AgentHostProfileForIdentity(%q) returned ok=false for a known token", token) + } + if got != profile.RuntimePlatform { + t.Fatalf( + "RuntimePlatformForHostIdentityToken(%q) = %q, but resolved profile.RuntimePlatform = %q", + token, got, profile.RuntimePlatform, + ) + } + if got == "" { + t.Fatalf("expected non-empty runtime platform for resolved token %q", token) + } + }) + } +} + +// TestBranchcov0720am_AgentHostProfileEntry_MatchesIdentity exercises every +// branch of the MatchesIdentity method (which delegates to +// agentHostProfileMatchesIdentity): +// - empty/whitespace input -> false, +// - match via the profile's own ID (exact and case/whitespace-tolerant), +// - match via one of the HostIdentityTokens, +// - non-match for an unrelated value, +// - non-match for a value that merely shares a substring with a token. +// +// The test constructs a fresh AgentHostProfileEntry value rather than relying +// on the global manifest so that we are exercising the method's contract on an +// arbitrary instance, not pinning a particular global entry. +func TestBranchcov0720am_AgentHostProfileEntry_MatchesIdentity(t *testing.T) { + profile := AgentHostProfileEntry{ + ID: "unraid", + HostIdentityTokens: []string{"unraid-os", "unraid os"}, + } + + cases := []struct { + name string + input string + want bool + }{ + {name: "exact id", input: "unraid", want: true}, + {name: "id case-insensitive", input: "UNRAID", want: true}, + {name: "id with surrounding whitespace", input: " unraid ", want: true}, + {name: "alias token with hyphen exact", input: "unraid-os", want: true}, + {name: "alias token with space", input: "unraid os", want: true}, + {name: "alias token mixed case and whitespace", input: " Unraid OS ", want: true}, + {name: "unrelated value", input: "ubuntu", want: false}, + {name: "partial substring of id is not a match", input: "unr", want: false}, + {name: "partial substring of a token is not a match", input: "unraid-o", want: false}, + {name: "empty input", input: "", want: false}, + {name: "whitespace-only input", input: " \t ", want: false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := profile.MatchesIdentity(tc.input); got != tc.want { + t.Fatalf("MatchesIdentity(%q) = %v, want %v", tc.input, got, tc.want) + } + }) + } +} + +// TestBranchcov0720am_AgentHostProfileEntry_MatchesIdentity_TokenlessProfile +// asserts that the ID-match branch is independent of HostIdentityTokens: a +// profile with no tokens at all still matches by its ID, and still rejects +// everything else. This is the only way to prove the loop branch is exercised +// in isolation from the ID shortcut. +func TestBranchcov0720am_AgentHostProfileEntry_MatchesIdentity_TokenlessProfile(t *testing.T) { + tokenless := AgentHostProfileEntry{ID: "solo"} + + if !tokenless.MatchesIdentity("solo") { + t.Fatal(`MatchesIdentity("solo") = false on tokenless profile, want true (id-match branch)`) + } + if !tokenless.MatchesIdentity(" Solo ") { + t.Fatal(`MatchesIdentity(" Solo ") = false on tokenless profile, want true (id-match is case/whitespace-tolerant)`) + } + if tokenless.MatchesIdentity("solo-clone") { + t.Fatal(`MatchesIdentity("solo-clone") = true on tokenless profile, want false (no tokens to match)`) + } + if tokenless.MatchesIdentity("") { + t.Fatal(`MatchesIdentity("") = true on tokenless profile, want false (empty input guard)`) + } +} + +// TestBranchcov0720am_AgentHostProfileEntry_MatchesIdentity_MatchesOnlyViaToken +// asserts the token-loop arm independently: a profile whose ID does not equal +// the input but which carries the input in its HostIdentityTokens must still +// report a match. This isolates the loop branch from the ID shortcut. +func TestBranchcov0720am_AgentHostProfileEntry_MatchesIdentity_MatchesOnlyViaToken(t *testing.T) { + profile := AgentHostProfileEntry{ + ID: "primary-id", + HostIdentityTokens: []string{"alias-one", "alias-two"}, + } + + if !profile.MatchesIdentity("alias-one") { + t.Fatal(`MatchesIdentity("alias-one") = false, want true (must match via HostIdentityTokens[0])`) + } + if !profile.MatchesIdentity(" Alias-Two ") { + t.Fatal(`MatchesIdentity(" Alias-Two ") = false, want true (must match via HostIdentityTokens[1] with normalisation)`) + } + if profile.MatchesIdentity("primary-id-extra") { + t.Fatal(`MatchesIdentity("primary-id-extra") = true, want false (substring of ID is not a match)`) + } + if profile.MatchesIdentity("alias") { + t.Fatal(`MatchesIdentity("alias") = true, want false (partial token substring is not a match)`) + } +} + +// TestBranchcov0720am_AgentHostProfiles covers AgentHostProfiles. The function +// is not branchless — it allocates and clones every entry — so we assert: +// - the returned slice is non-empty and matches the manifest length, +// - every entry has a non-empty ID (self-consistency of the projection), +// - mutating the returned slice (and its nested HostIdentityTokens) does not +// bleed into subsequent calls — i.e. the function returns an independent +// copy, which is the whole point of the clone logic. +func TestBranchcov0720am_AgentHostProfiles(t *testing.T) { + first := AgentHostProfiles() + if len(first) == 0 { + t.Fatal("AgentHostProfiles() returned an empty slice, expected at least one projected entry") + } + + // Snapshot the IDs and first-token of every entry to compare against a + // later call. We deliberately do not assert against an exact count or + // exact literals — only that the second call is consistent with the first + // and that mutations we introduce cannot be observed. + type fingerprint struct { + id string + firstToken string + tokenCount int + runtimePlatID string + } + snapshot := make([]fingerprint, len(first)) + for i, entry := range first { + if entry.ID == "" { + t.Fatalf("AgentHostProfiles()[%d].ID is empty; projection must always carry an id", i) + } + firstToken := "" + if len(entry.HostIdentityTokens) > 0 { + firstToken = entry.HostIdentityTokens[0] + } + snapshot[i] = fingerprint{ + id: entry.ID, + firstToken: firstToken, + tokenCount: len(entry.HostIdentityTokens), + runtimePlatID: entry.RuntimePlatform, + } + } + + // Mutate the first call's contents aggressively: drop entries, rewrite + // fields, and corrupt the nested token slice. None of this should be + // visible to a fresh call. + if len(first[0].HostIdentityTokens) > 0 { + first[0].HostIdentityTokens[0] = "__mutated_token__" + } + first[0].ID = "__mutated_id__" + first[0].RuntimePlatform = "__mutated_platform__" + first = first[:0] + + second := AgentHostProfiles() + if len(second) != len(snapshot) { + t.Fatalf("AgentHostProfiles() length changed between calls: first=%d, second=%d", len(snapshot), len(second)) + } + for i, entry := range second { + want := snapshot[i] + if entry.ID != want.id { + t.Fatalf("AgentHostProfiles()[%d].ID changed between calls: got %q, want %q (mutation leaked)", i, entry.ID, want.id) + } + if entry.RuntimePlatform != want.runtimePlatID { + t.Fatalf("AgentHostProfiles()[%d].RuntimePlatform changed between calls: got %q, want %q", i, entry.RuntimePlatform, want.runtimePlatID) + } + if len(entry.HostIdentityTokens) != want.tokenCount { + t.Fatalf("AgentHostProfiles()[%d].HostIdentityTokens length changed: got %d, want %d", i, len(entry.HostIdentityTokens), want.tokenCount) + } + if want.firstToken != "" && len(entry.HostIdentityTokens) > 0 && entry.HostIdentityTokens[0] != want.firstToken { + t.Fatalf("AgentHostProfiles()[%d].HostIdentityTokens[0] changed: got %q, want %q (nested mutation leaked)", i, entry.HostIdentityTokens[0], want.firstToken) + } + } +} + +// TestBranchcov0720am_AgentHostProfiles_IndependentSliceAliasing is a sharper +// isolation check: two back-to-back calls must not share backing arrays for +// either the outer slice or any inner HostIdentityTokens slice. Appending to +// one result must never affect the other. +func TestBranchcov0720am_AgentHostProfiles_IndependentSliceAliasing(t *testing.T) { + a := AgentHostProfiles() + b := AgentHostProfiles() + if len(a) == 0 || len(b) == 0 { + t.Fatal("AgentHostProfiles() returned empty slice; cannot exercise aliasing check") + } + + // Append a synthetic entry to `a` and verify `b` is unaffected. + synthetic := AgentHostProfileEntry{ID: "__synthetic__"} + a = append(a, synthetic) + + if len(b) != len(a)-1 { + t.Fatalf("append to one AgentHostProfiles() result leaked into a fresh call: a=%d, b=%d", len(a), len(b)) + } + for _, entry := range b { + if entry.ID == synthetic.ID { + t.Fatalf("synthetic entry leaked into fresh AgentHostProfiles() result: %+v", entry) + } + } + + // Mutate an inner token slice of one entry in `b` and confirm `a`'s + // equivalent entry is unaffected (note: `a[len(a)-1]` is the synthetic + // entry, so we compare against the first real entry which still exists at + // index 0 in both results). + if len(b[0].HostIdentityTokens) > 0 && len(a[0].HostIdentityTokens) > 0 { + originalA := a[0].HostIdentityTokens[0] + b[0].HostIdentityTokens[0] = "__corrupt_via_b__" + if a[0].HostIdentityTokens[0] != originalA { + t.Fatalf("mutating b[0].HostIdentityTokens[0] changed a[0].HostIdentityTokens[0]: got %q, want %q (inner slice aliasing)", a[0].HostIdentityTokens[0], originalA) + } + } +} diff --git a/internal/unifiedresources/actions_branchcov0720am_test.go b/internal/unifiedresources/actions_branchcov0720am_test.go new file mode 100644 index 000000000..c4b14775d --- /dev/null +++ b/internal/unifiedresources/actions_branchcov0720am_test.go @@ -0,0 +1,429 @@ +package unifiedresources + +import ( + "errors" + "fmt" + "testing" + "time" +) + +// Branch-coverage tests for currently-uncovered functions in actions.go: +// - IsPermanentActionExecutionRefusal (error classifier; nil, wrapped +// sentinels, unrelated error) +// - ValidateHumanActionBinding (validation; each rejection arm + +// the valid path) +// +// ActionPolicyAuthorizationDigest is intentionally NOT exercised here: its +// body is straight-line (zero Digest, json.Marshal, sha256.Sum256, Sprintf) +// with no conditional logic to drive, so it has no branches to cover. See +// GLM_REPORT.md for the skip rationale. + +// --------------------------------------------------------------------------- +// IsPermanentActionExecutionRefusal +// --------------------------------------------------------------------------- + +// TestBranchcov0720am_IsPermanentActionExecutionRefusal drives every arm of +// the underlying permanentActionExecutionRefusalMessage switch via the public +// classifier: each permanent sentinel (direct and wrapped), nil, an unrelated +// error, and the non-permanent ErrActionExecutionRefusal wrapper. +func TestBranchcov0720am_IsPermanentActionExecutionRefusal(t *testing.T) { + permanentSentinels := []struct { + name string + err error + }{ + {"plan drift", ErrActionPlanDrift}, + {"plan expired", ErrActionPlanExpired}, + {"dry run only", ErrActionDryRunOnly}, + {"resource remediation locked", ErrResourceRemediationLocked}, + {"policy authorization expired", ErrActionPolicyAuthorizationExpired}, + {"policy authorization invalid", ErrActionPolicyAuthorizationInvalid}, + {"policy authorization revoked", ErrActionPolicyAuthorizationRevoked}, + {"emergency stop", ErrActionEmergencyStop}, + {"replan required", ErrActionReplanRequired}, + } + + for _, s := range permanentSentinels { + s := s + t.Run("sentinel/"+s.name, func(t *testing.T) { + if !IsPermanentActionExecutionRefusal(s.err) { + t.Errorf("IsPermanentActionExecutionRefusal(%v) = false, want true", s.err) + } + }) + t.Run("wrapped/"+s.name, func(t *testing.T) { + wrapped := fmt.Errorf("transport failure: %w", s.err) + if !IsPermanentActionExecutionRefusal(wrapped) { + t.Errorf("IsPermanentActionExecutionRefusal(wrapped %v) = false, want true", s.err) + } + }) + } + + t.Run("nil error", func(t *testing.T) { + if IsPermanentActionExecutionRefusal(nil) { + t.Error("IsPermanentActionExecutionRefusal(nil) = true, want false") + } + }) + + t.Run("unrelated error", func(t *testing.T) { + if IsPermanentActionExecutionRefusal(errors.New("transient network blip")) { + t.Error("IsPermanentActionExecutionRefusal(unrelated) = true, want false") + } + }) + + // ErrActionExecutionRefusal is the wrapper RefuseActionExecution returns + // when the reason is NOT a permanent refusal. It must therefore not + // classify as permanent itself, or transient errors would be mis-reported. + t.Run("execution refusal wrapper is not permanent", func(t *testing.T) { + if IsPermanentActionExecutionRefusal(ErrActionExecutionRefusal) { + t.Error("ErrActionExecutionRefusal must not be permanent (it is the non-permanent wrapper sentinel)") + } + }) +} + +// --------------------------------------------------------------------------- +// ValidateHumanActionBinding +// --------------------------------------------------------------------------- + +// TestBranchcov0720am_ValidateHumanActionBinding drives each rejection arm of +// the binding validator (first-guard disjuncts, inner-block continue arms, +// quorum-not-met) plus the happy paths that return nil. +func TestBranchcov0720am_ValidateHumanActionBinding(t *testing.T) { + now := time.Date(2026, 7, 20, 9, 0, 0, 0, time.UTC) + const validOrg = "default" + + validActor := func(orgID string) ActionActor { + return ActionActor{SubjectID: "agent:helper", Kind: ActionActorService, CredentialID: "service:test", OrgID: orgID} + } + + // baseRecord builds a record whose Request.Actor and Plan are coherent + // under ValidateHumanActionBinding's first guard. Callers mutate the + // specific field needed to drive a particular arm. + baseRecord := func(state ActionState, requiresApproval bool, policy ActionApprovalLevel) ActionAuditRecord { + return ActionAuditRecord{ + ID: "action-1", + State: state, + Request: ActionRequest{ + Actor: validActor(validOrg), + }, + Plan: ActionPlan{ + PlanHash: "sha256:test", + RequiresApproval: requiresApproval, + ApprovalPolicy: policy, + ApprovalRequirement: ApprovalRequirementForFloor(policy), + }, + } + } + + // boundApproval builds an approval whose ActorBinding and Evidence are + // mutually coherent and coherent with the record, so it counts toward + // quorum by default. Callers mutate fields to drive negative arms. + boundApproval := func(record ActionAuditRecord, subject string, kind ActionActorKind, method ApprovalMethod) ActionApprovalRecord { + binding := ActionActor{SubjectID: subject, Kind: kind, CredentialID: string(kind) + ":test", OrgID: validOrg} + evidence := ApprovalEvidence{ + Version: 1, + Method: method, + Actor: binding, + OrgID: validOrg, + ActionID: record.ID, + PlanHash: record.Plan.PlanHash, + Outcome: OutcomeApproved, + IssuedAt: now, + } + return ActionApprovalRecord{ + Actor: subject, + ActorBinding: binding, + Method: method, + Timestamp: now, + Outcome: OutcomeApproved, + Evidence: &evidence, + } + } + + cases := []struct { + name string + record ActionAuditRecord + orgID string + wantErr error // nil expects no error; otherwise must satisfy errors.Is + }{ + // --- First guard: each disjunct independently returns ErrActionReplanRequired --- + { + name: "first guard rejects invalid request actor (missing subject)", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + r.Request.Actor = ActionActor{SubjectID: "", Kind: ActionActorUser, CredentialID: "x", OrgID: validOrg} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "first guard rejects org id mismatch", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + r.Request.Actor = validActor("other-org") + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "first guard rejects requirement version mismatch", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + r.Plan.ApprovalRequirement.Version = 2 // unsupported version + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "first guard rejects requirement floor diverging from policy", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + // Non-empty Floor different from ApprovalPolicy; NormalizeApprovalRequirement + // will not backfill it from the policy, so the divergence survives. + r.Plan.ApprovalRequirement.Floor = ApprovalNone + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + + // --- Inner binding block is skipped: returns nil --- + { + name: "no approval required skips binding check", + record: baseRecord(ActionStatePlanned, false, ApprovalNone), + orgID: validOrg, + wantErr: nil, + }, + { + name: "approval required but state is Pending skips binding check", + record: baseRecord(ActionStatePending, true, ApprovalAdmin), + orgID: validOrg, + wantErr: nil, + }, + + // --- Inner block entered; quorum not met -> ErrActionReplanRequired --- + { + name: "approved but zero approvals collected", + record: baseRecord(ActionStateApproved, true, ApprovalAdmin), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "rejected approval outcome is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Outcome = OutcomeRejected + ap.Evidence.Outcome = OutcomeRejected + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "approved outcome with nil evidence is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Evidence = nil + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "invalid actor binding (missing subject) is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.ActorBinding.SubjectID = "" + ap.Evidence.Actor.SubjectID = "" + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "actor binding/evidence actor mismatch is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Evidence.Actor = ActionActor{SubjectID: "someone-else", Kind: ActionActorUser, CredentialID: "x", OrgID: validOrg} + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "evidence version not 1 is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Evidence.Version = 2 + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "evidence org id mismatch is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Evidence.OrgID = "other-org" + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "evidence action id mismatch is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Evidence.ActionID = "other-action" + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "evidence plan hash mismatch is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Evidence.PlanHash = "sha256:different" + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "evidence outcome not approved is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + // approval.Outcome stays Approved so the first if proceeds; only + // the evidence outcome disjunct in the second if triggers. + ap.Evidence.Outcome = OutcomeRejected + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "evidence issued-at zero is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + ap.Evidence.IssuedAt = time.Time{} + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "non-MFA floor with non-session/token method is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + // MethodWebAuthnUV is neither Session nor APIToken, so the + // non-MFA else-branch's continue fires. + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodWebAuthnUV) + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + { + name: "MFA floor with non-WebAuthn/DeviceKey method is ignored", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalMultiFactor) + // MethodSession is neither WebAuthnUV nor DeviceKeyUV, so the + // MFA if-branch's continue fires. + ap := boundApproval(r, "op@example.com", ActionActorUser, MethodSession) + r.Approvals = []ActionApprovalRecord{ap} + return r + }(), + orgID: validOrg, + wantErr: ErrActionReplanRequired, + }, + + // --- Happy paths inside the inner block: returns nil --- + { + name: "non-MFA valid session approval passes", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + r.Approvals = []ActionApprovalRecord{ + boundApproval(r, "op@example.com", ActionActorUser, MethodSession), + } + return r + }(), + orgID: validOrg, + wantErr: nil, + }, + { + name: "non-MFA valid API token approval passes", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalAdmin) + r.Approvals = []ActionApprovalRecord{ + boundApproval(r, "auto-bot", ActionActorAPIToken, MethodAPIToken), + } + return r + }(), + orgID: validOrg, + wantErr: nil, + }, + { + name: "MFA valid WebAuthn approval passes", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateApproved, true, ApprovalMultiFactor) + r.Approvals = []ActionApprovalRecord{ + boundApproval(r, "op@example.com", ActionActorUser, MethodWebAuthnUV), + } + return r + }(), + orgID: validOrg, + wantErr: nil, + }, + { + name: "MFA valid device-key approval passes and exercises Executing state", + record: func() ActionAuditRecord { + r := baseRecord(ActionStateExecuting, true, ApprovalMultiFactor) + r.Approvals = []ActionApprovalRecord{ + boundApproval(r, "op@example.com", ActionActorUser, MethodDeviceKeyUV), + } + return r + }(), + orgID: validOrg, + wantErr: nil, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := ValidateHumanActionBinding(tc.record, tc.orgID) + switch { + case tc.wantErr == nil: + if err != nil { + t.Errorf("ValidateHumanActionBinding() unexpected error: %v", err) + } + case err == nil: + t.Errorf("ValidateHumanActionBinding() returned nil, want error matching %v", tc.wantErr) + case !errors.Is(err, tc.wantErr): + t.Errorf("ValidateHumanActionBinding() error = %v, want %v", err, tc.wantErr) + } + }) + } +} diff --git a/internal/unifiedresources/availability_branchcov0720am_test.go b/internal/unifiedresources/availability_branchcov0720am_test.go new file mode 100644 index 000000000..2a8e90283 --- /dev/null +++ b/internal/unifiedresources/availability_branchcov0720am_test.go @@ -0,0 +1,192 @@ +package unifiedresources + +import ( + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/operationaltrust" +) + +// These tests augment availability_test.go with branch-coverage assertions +// for AvailabilityCheckByTargetID for the 0720am coverage pass. They focus on +// the three conditional arms of that lookup: found-returns-clone, +// not-found-returns-nil, and the empty/nil-data arm where the resource has no +// checks at all (so the loop body never executes). Independence of the +// returned clone is also asserted so future refactors cannot silently begin +// returning aliases into the resource's stored state. + +func TestBranchcov0720am_AvailabilityCheckByTargetID(t *testing.T) { + checkedAt := time.Date(2026, time.July, 20, 8, 0, 0, 0, time.UTC) + + cases := []struct { + name string + resource Resource + targetID string + wantFound bool + wantTarget string // asserted verbatim only when wantFound is true + }{ + { + name: "found: exact TargetID in AvailabilityChecks returns a clone", + resource: Resource{AvailabilityChecks: []AvailabilityData{ + {TargetID: "switch-1", Available: false}, + {TargetID: "router-1", Available: true, LastChecked: &checkedAt}, + }}, + targetID: "router-1", + wantFound: true, + wantTarget: "router-1", + }, + { + name: "found: caller-supplied targetID with surrounding whitespace is trimmed before matching", + resource: Resource{AvailabilityChecks: []AvailabilityData{ + {TargetID: "router-1", Available: true}, + }}, + targetID: " router-1\t", + wantFound: true, + wantTarget: "router-1", + }, + { + // Both sides trim: a check whose stored TargetID has whitespace + // still matches a clean caller input. The stored value is not + // rewritten by merge, so we assert on the trimmed form rather than + // pinning the exact whitespace. + name: "found: stored check TargetID with surrounding whitespace matches after trim on both sides", + resource: Resource{AvailabilityChecks: []AvailabilityData{ + {TargetID: " router-1 "}, + }}, + targetID: "router-1", + wantFound: true, + }, + { + name: "not found: checks present but no TargetID matches", + resource: Resource{AvailabilityChecks: []AvailabilityData{ + {TargetID: "router-1"}, + {TargetID: "switch-1"}, + }}, + targetID: "missing-target", + wantFound: false, + }, + { + name: "nil-data: resource has no checks and no compatibility summary -> loop never executes -> nil", + resource: Resource{}, + targetID: "router-1", + wantFound: false, + }, + { + name: "empty after trim: whitespace-only targetID must not match any stored check", + resource: Resource{AvailabilityChecks: []AvailabilityData{ + {TargetID: "router-1"}, + }}, + targetID: " ", + wantFound: false, + }, + { + // The compatibility summary (resource.Availability) is folded into + // the canonical check set by AvailabilityChecksForResource, so a + // probe recorded only as the singular summary must also be + // retrievable by TargetID. This is the alert/evidence consumer + // contract the function's doc comment promises. + name: "found: singular Availability summary is folded in and matchable by TargetID", + resource: Resource{ + Availability: &AvailabilityData{TargetID: "summary-target", Available: true}, + }, + targetID: "summary-target", + wantFound: true, + wantTarget: "summary-target", + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + got := AvailabilityCheckByTargetID(c.resource, c.targetID) + if c.wantFound { + if got == nil { + t.Fatalf("AvailabilityCheckByTargetID(...) = nil, want non-nil clone for targetID %q", c.targetID) + } + if c.wantTarget != "" && got.TargetID != c.wantTarget { + t.Errorf("returned TargetID = %q, want %q", got.TargetID, c.wantTarget) + } + } else { + if got != nil { + t.Fatalf("AvailabilityCheckByTargetID(...) = %+v, want nil", got) + } + } + }) + } +} + +func TestBranchcov0720am_AvailabilityCheckByTargetID_CloneIsIndependent(t *testing.T) { + // The found arm must return a deep clone: mutating scalar fields, the + // time-pointer fields, and the Evidence envelope of the returned value + // must not propagate back into the resource's stored check. A future + // refactor that began returning an alias would surface here. + originalChecked := time.Date(2026, time.July, 20, 8, 0, 0, 0, time.UTC) + source := operationaltrust.EvidenceEnvelope{ + Source: operationaltrust.EvidenceSource{Provider: "availability", Collector: "poller"}, + Subject: operationaltrust.EvidenceSubject{ProviderRef: "router-1", ProviderScope: "availability-target"}, + ObservedAt: originalChecked, + Completeness: operationaltrust.EvidenceComplete, + Confidence: operationaltrust.EvidenceConfirmed, + } + resource := Resource{AvailabilityChecks: []AvailabilityData{{ + TargetID: "router-1", + Available: true, + LastChecked: &originalChecked, + Evidence: &source, + }}} + + got := AvailabilityCheckByTargetID(resource, "router-1") + if got == nil { + t.Fatalf("AvailabilityCheckByTargetID(...) = nil, want clone") + } + + // Mutate every cloneable field of the returned value. + got.Available = false + got.LastChecked = nil + if got.Evidence != nil { + got.Evidence.Completeness = operationaltrust.EvidencePartial + got.Evidence.Source.Provider = "mutated" + } + + again := AvailabilityCheckByTargetID(resource, "router-1") + if again == nil { + t.Fatalf("second lookup returned nil; resource state was mutated by the first call") + } + if !again.Available { + t.Errorf("scalar field independence: Available = false, want true (clone leaked mutation into source)") + } + if again.LastChecked == nil || !again.LastChecked.Equal(originalChecked) { + t.Errorf("LastChecked independence: got %v, want %v (pointer field shared with source)", again.LastChecked, originalChecked) + } + if again.Evidence == nil { + t.Fatalf("Evidence independence: Evidence = nil, want non-nil clone") + } + if again.Evidence.Completeness != operationaltrust.EvidenceComplete { + t.Errorf("Evidence independence: Completeness = %q, want %q (envelope shared with source)", + again.Evidence.Completeness, operationaltrust.EvidenceComplete) + } + if again.Evidence.Source.Provider != "availability" { + t.Errorf("Evidence independence: Source.Provider = %q, want %q (nested struct shared with source)", + again.Evidence.Source.Provider, "availability") + } +} + +func TestBranchcov0720am_AvailabilityCheckByTargetID_FirstMatchWins(t *testing.T) { + // When two distinct checks would normalize to the same TargetID, the + // canonical merge dedupes them; the lookup must therefore find exactly + // one entry and return it without panic. This pins the "first match" + // semantics against any future change that began scanning into a + // non-deduped slice. + resource := Resource{AvailabilityChecks: []AvailabilityData{ + {TargetID: "router-1", Available: true}, + {TargetID: "router-1", Available: false}, // duplicate key, merged away + }} + + got := AvailabilityCheckByTargetID(resource, "router-1") + if got == nil { + t.Fatalf("AvailabilityCheckByTargetID(...) = nil, want the single merged check") + } + // After merge only one entry exists, so the lookup is unambiguous; we + // only assert presence here, not which duplicate survived, because the + // merge order is an internal detail of mergeAvailabilityChecks. +} diff --git a/internal/unifiedresources/capabilities_branchcov0720am_test.go b/internal/unifiedresources/capabilities_branchcov0720am_test.go new file mode 100644 index 000000000..de22a6e4e --- /dev/null +++ b/internal/unifiedresources/capabilities_branchcov0720am_test.go @@ -0,0 +1,172 @@ +package unifiedresources + +import "testing" + +// Branch-coverage tests for the ActionAutoAuthorizationClass validators in +// capabilities.go. +// +// Two target functions, each with genuine conditional logic: +// +// - IsValidActionAutoAuthorizationClass: a switch with a true-arm (empty + +// the three canonical classes) and a default false-arm. +// - NormalizeActionAutoAuthorizationClass: an `||` short-circuit with three +// distinct arms — empty input, non-empty invalid input, and valid +// pass-through. +// +// Assertions are behavioural: for the validator we assert the boolean +// classification across each arm; for the normalizer we assert the +// fallback-vs-passthrough contract (fallback coerces the value away from the +// input and to the canonical "never" sentinel; pass-through preserves the +// input value exactly). We never pin a literal string — only the published +// constants and the input/output relationship. + +// TestBranchcov0720am_IsValidActionAutoAuthorizationClass exercises every +// case-arm of the validator's switch: the empty string, each canonical class +// constant, and several representative invalid values (a totally bogus value, +// a permissive-sounding-but-unknown value, a wrong-case near-miss, and an +// untrimmed variant of a valid value). +func TestBranchcov0720am_IsValidActionAutoAuthorizationClass(t *testing.T) { + tests := []struct { + name string + in ActionAutoAuthorizationClass + want bool + }{ + {name: "empty is valid", in: "", want: true}, + {name: "never is valid", in: AutoAuthorizeNever, want: true}, + {name: "low_risk is valid", in: AutoAuthorizeLowRisk, want: true}, + {name: "elevated is valid", in: AutoAuthorizeElevated, want: true}, + {name: "bogus high_risk is invalid", in: "high_risk", want: false}, + {name: "permissive-sounding always is invalid", in: "always", want: false}, + {name: "wrong-case NEVER is invalid (case-sensitive)", in: "NEVER", want: false}, + {name: "untrimmed low_risk is invalid (no implicit trim)", in: " low_risk ", want: false}, + {name: "arbitrary non-empty garbage is invalid", in: "auto", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IsValidActionAutoAuthorizationClass(tt.in) + if got != tt.want { + t.Fatalf("IsValidActionAutoAuthorizationClass(%q) = %v, want %v", tt.in, got, tt.want) + } + }) + } +} + +// TestBranchcov0720am_NormalizeActionAutoAuthorizationClass exercises all +// three control-flow arms of the normalizer: +// +// - empty input -> fallback (AutoAuthorizeNever), and the result is +// non-empty (proving coercion occurred rather than pass-through of ""). +// - non-empty invalid input -> fallback (AutoAuthorizeNever), and the +// result differs from the input (proving coercion rather than pass-through). +// - each canonical non-empty valid input -> the exact same value is +// returned (pass-through / identity), which is the discriminative proof +// that the function does not blindly collapse everything to "never". +func TestBranchcov0720am_NormalizeActionAutoAuthorizationClass(t *testing.T) { + tests := []struct { + name string + in ActionAutoAuthorizationClass + want ActionAutoAuthorizationClass + coerced bool // true iff the normalizer must change the value (fallback arms) + discrimAPI bool // true iff want is a distinct canonical used to discriminate pass-through + }{ + {name: "empty input falls back to never", in: "", want: AutoAuthorizeNever, coerced: true}, + {name: "bogus high_risk falls back to never", in: "high_risk", want: AutoAuthorizeNever, coerced: true}, + {name: "permissive-sounding always falls back to never", in: "always", want: AutoAuthorizeNever, coerced: true}, + {name: "wrong-case NEVER falls back to never", in: "NEVER", want: AutoAuthorizeNever, coerced: true}, + {name: "never passes through", in: AutoAuthorizeNever, want: AutoAuthorizeNever, discrimAPI: true}, + {name: "low_risk passes through unchanged", in: AutoAuthorizeLowRisk, want: AutoAuthorizeLowRisk, discrimAPI: true}, + {name: "elevated passes through unchanged", in: AutoAuthorizeElevated, want: AutoAuthorizeElevated, discrimAPI: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := NormalizeActionAutoAuthorizationClass(tt.in) + if got != tt.want { + t.Fatalf("NormalizeActionAutoAuthorizationClass(%q) = %q, want %q", tt.in, got, tt.want) + } + // Invariant: the normalizer never returns an empty string. + // This proves the empty-input arm coerces rather than passing "" through. + if got == "" { + t.Fatalf("NormalizeActionAutoAuthorizationClass(%q) returned empty; normalizer must always emit a non-empty canonical", tt.in) + } + if tt.coerced { + // On the fallback arms the result must differ from the + // (non-canonical or empty) input — this is what distinguishes + // a real fallback from a no-op pass-through. + if got == tt.in { + t.Fatalf("NormalizeActionAutoAuthorizationClass(%q): expected coercion but result equals input %q", tt.in, got) + } + } + if tt.discrimAPI { + // On the pass-through arms the result must equal the input + // exactly — this proves the function is not collapsing every + // valid input onto "never". + if got != tt.in { + t.Fatalf("NormalizeActionAutoAuthorizationClass(%q): expected identity pass-through but got %q", tt.in, got) + } + } + }) + } +} + +// TestBranchcov0720am_NormalizeActionAutoAuthorizationClass_FallbackConsistency +// asserts the behavioural contract that every invalid/empty input collapses to +// the SAME canonical sentinel as an explicit AutoAuthorizeNever input. This is +// the user-facing guarantee ("unknown always means never") and is asserted by +// comparing outputs of the function under test against each other rather than +// by re-stating a literal. +func TestBranchcov0720am_NormalizeActionAutoAuthorizationClass_FallbackConsistency(t *testing.T) { + neverResult := NormalizeActionAutoAuthorizationClass(AutoAuthorizeNever) + if neverResult == "" { + t.Fatalf("baseline: Normalize(never) is empty, want the never canonical") + } + + invalidInputs := []ActionAutoAuthorizationClass{ + "", + "high_risk", + "always", + "NEVER", + " low_risk ", + "auto", + "unknown", + } + for _, in := range invalidInputs { + got := NormalizeActionAutoAuthorizationClass(in) + if got != neverResult { + t.Errorf("Normalize(%q) = %q, want same canonical as Normalize(never)=%q (unknown must always mean never)", in, got, neverResult) + } + // And the result must differ from the (non-canonical) input where the + // input is non-empty, proving the value was actually coerced. + if in != "" && got == in { + t.Errorf("Normalize(%q) returned the input unchanged; invalid input must be coerced to never", in) + } + } +} + +// TestBranchcov0720am_NormalizeActionAutoAuthorizationClass_PassThroughPreservesDistinct +// is the positive discriminative test for the pass-through arm: feeding the two +// non-"never" canonical classes must preserve them distinctly. If the +// normalizer were buggy and collapsed everything to "never", low_risk and +// elevated would compare equal — this test would fail. +func TestBranchcov0720am_NormalizeActionAutoAuthorizationClass_PassThroughPreservesDistinct(t *testing.T) { + low := NormalizeActionAutoAuthorizationClass(AutoAuthorizeLowRisk) + elev := NormalizeActionAutoAuthorizationClass(AutoAuthorizeElevated) + never := NormalizeActionAutoAuthorizationClass(AutoAuthorizeNever) + + if low == elev { + t.Fatalf("low_risk and elevated normalised to the same value %q; pass-through must preserve distinct canonicals", low) + } + if low == never { + t.Fatalf("low_risk normalised to never %q; pass-through must preserve low_risk as distinct from never", low) + } + if elev == never { + t.Fatalf("elevated normalised to never %q; pass-through must preserve elevated as distinct from never", elev) + } + if low != AutoAuthorizeLowRisk { + t.Fatalf("low_risk pass-through: got %q, want low_risk", low) + } + if elev != AutoAuthorizeElevated { + t.Fatalf("elevated pass-through: got %q, want elevated", elev) + } +} diff --git a/internal/unifiedresources/host_apt_telemetry_branchcov0720am_test.go b/internal/unifiedresources/host_apt_telemetry_branchcov0720am_test.go new file mode 100644 index 000000000..a93fe86cf --- /dev/null +++ b/internal/unifiedresources/host_apt_telemetry_branchcov0720am_test.go @@ -0,0 +1,183 @@ +package unifiedresources + +import ( + "strings" + "testing" +) + +// Branch-coverage tests for ValidHostAPTDigest in host_apt_telemetry.go. +// +// The validator normalizes with TrimSpace, then guards on length AND a +// "sha256:" prefix (combined in a single short-circuiting OR), then accepts +// only if the trailing 64 bytes are lowercase-hex-decodable AND the whole +// token is already lowercase. The table below drives every distinct arm: +// +// - empty / whitespace-only input (length arm via TrimSpace normalization) +// - leading+trailing whitespace around an otherwise valid token (TrimSpace) +// - correct length but wrong prefix (prefix arm of the OR, length passes) +// - correct prefix but length too short / too long (length arm of the OR) +// - both length and prefix wrong (OR side that proves the second operand +// can also drive the false return when the first is true) +// - correct shape but uppercase hex letters (ToLower arm, decode succeeds) +// - correct shape but mixed-case hex (ToLower arm still rejects) +// - correct shape but non-hex characters in body (hex.DecodeString err arm) +// - canonical happy path: lowercase-hex 64-char digest, valid prefix/length + +// hexLower builds a lowercase hex body of the requested length using a +// deterministic repeating pattern so the test does not depend on importing +// any source-side constant. +func hexLower(n int) string { + const src = "0123456789abcdef" + var b strings.Builder + for i := 0; i < n; i++ { + b.WriteByte(src[i%len(src)]) + } + return b.String() +} + +// hexUpper mirrors hexLower but uses A-F so the body decodes successfully yet +// fails the lowercase identity check. +func hexUpper(n int) string { + const src = "0123456789ABCDEF" + var b strings.Builder + for i := 0; i < n; i++ { + b.WriteByte(src[i%len(src)]) + } + return b.String() +} + +func TestBranchcov0720am_ValidHostAPTDigest(t *testing.T) { + // Reference lowercase 64-char hex body. + lower64 := hexLower(64) + upper64 := hexUpper(64) + // Non-hex body of correct length: 'z' is not a hex digit. + nonHex64 := strings.Repeat("z", 64) + + tests := []struct { + name string + value string + want bool + }{ + // --- TrimSpace normalization arm (line 12) --- + { + name: "empty string fails length arm after trim", + value: "", + want: false, + }, + { + name: "whitespace-only collapses to empty and fails length arm", + value: " \t\n", + want: false, + }, + { + name: "leading and trailing whitespace around valid token is trimmed and accepted", + value: " sha256:" + lower64 + " \t", + want: true, + }, + + // --- Length arm of the OR guard (line 13, left operand) --- + { + name: "correct prefix but body too short fails length arm", + value: "sha256:" + hexLower(63), + want: false, + }, + { + name: "correct prefix but body too long fails length arm", + value: "sha256:" + hexLower(65), + want: false, + }, + + // --- Prefix arm of the OR guard (line 13, right operand) --- + { + name: "correct length but wrong prefix fails prefix arm", + value: "sha512:" + lower64, + want: false, + }, + { + name: "correct length but missing prefix fails prefix arm", + value: lower64, + want: false, + }, + + // --- Both operands of the OR true (line 13) --- + { + name: "wrong prefix and wrong length fails composite guard", + value: "md5:" + hexLower(16), + want: false, + }, + + // --- hex.DecodeString error arm (line 16-17) --- + { + name: "correct shape but non-hex characters in body fails decode arm", + value: "sha256:" + nonHex64, + want: false, + }, + + // --- ToLower identity check arm (line 17, right operand) --- + { + name: "correct shape but uppercase hex body fails lowercase identity arm", + value: "sha256:" + upper64, + want: false, + }, + { + name: "correct shape but mixed-case hex body fails lowercase identity arm", + value: "sha256:" + hexLower(32) + hexUpper(32), + want: false, + }, + + // --- Happy path (line 17 returns true) --- + { + name: "canonical lowercase sha256 digest is accepted", + value: "sha256:" + lower64, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidHostAPTDigest(tt.value) + if got != tt.want { + t.Fatalf("ValidHostAPTDigest(%q) = %v, want %v", tt.value, got, tt.want) + } + }) + } +} + +// TestBranchcov0720am_ValidHostAPTDigest_Pure asserts behavioral invariants +// of the validator that hold regardless of the exact internal expression: +// the result must be deterministic for repeated calls, and TrimSpace +// equivalence must hold for any token the caller passes. +func TestBranchcov0720am_ValidHostAPTDigest_Invariants(t *testing.T) { + t.Run("deterministic across repeated calls for valid input", func(t *testing.T) { + v := "sha256:" + hexLower(64) + first := ValidHostAPTDigest(v) + for i := 0; i < 5; i++ { + if got := ValidHostAPTDigest(v); got != first { + t.Fatalf("non-deterministic result on call %d: got %v, want %v", i, got, first) + } + } + if !first { + t.Fatalf("expected canonical digest to validate, got %v", first) + } + }) + + t.Run("trimspace equivalent token yields same classification", func(t *testing.T) { + bare := "sha256:" + hexLower(64) + padded := "\t " + bare + " \n" + if ValidHostAPTDigest(bare) != ValidHostAPTDigest(padded) { + t.Fatalf("expected identical classification for bare and padded forms; bare=%v padded=%v", + ValidHostAPTDigest(bare), ValidHostAPTDigest(padded)) + } + }) + + t.Run("uppercase variant is never accepted when lowercase is", func(t *testing.T) { + lower := "sha256:" + hexLower(64) + upper := "sha256:" + hexUpper(64) + if !ValidHostAPTDigest(lower) { + t.Fatalf("precondition: lowercase digest must validate") + } + if ValidHostAPTDigest(upper) { + t.Fatalf("uppercase variant must be rejected by lowercase identity check") + } + }) +} diff --git a/internal/unifiedresources/monitored_system_projection_branchcov0720am_test.go b/internal/unifiedresources/monitored_system_projection_branchcov0720am_test.go new file mode 100644 index 000000000..510811e9f --- /dev/null +++ b/internal/unifiedresources/monitored_system_projection_branchcov0720am_test.go @@ -0,0 +1,521 @@ +package unifiedresources + +import "testing" + +// Branch-coverage tests for the four per-source selector predicates in +// monitored_system_projection.go: +// +// - monitoredSystemReplacementSelectorMatchesAgent +// - monitoredSystemReplacementSelectorMatchesProxmox +// - monitoredSystemReplacementSelectorMatchesPMG +// - monitoredSystemReplacementSelectorMatchesK8s +// +// Each predicate has (a) a nil-source-data early-return guard and (b) a +// short-circuit OR chain over several selector arms. To get full branch +// coverage of the OR chain we drive one sub-case per arm in which ONLY that +// arm matches (all earlier arms evaluate to false), plus a no-match case +// (every arm false) and the nil-source-data guard. +// +// All assertions are behavioural: they assert the returned boolean for +// representative inputs that drive each branch. No change-detector +// string-pinning, no re-imported constants. + +// --------------------------------------------------------------------------- +// shared fixtures +// --------------------------------------------------------------------------- + +// agentMatchResource is a populated Agent-backed Resource used as the +// non-matching base; each selector arm overrides exactly one field to drive +// only that arm to true. +func agentMatchResource() Resource { + return Resource{ + ID: "agent-res-1", + Name: "agent-row-name", + Agent: &AgentData{ + AgentID: "agent-1", + Hostname: "lab-a.example", + }, + Identity: ResourceIdentity{ + MachineID: "machine-1", + }, + } +} + +func proxmoxMatchResource() Resource { + return Resource{ + ID: "px-res-1", + Name: "px-row-name", + Proxmox: &ProxmoxData{ + SourceID: "px-src-1", + Instance: "px-instance-1", + NodeName: "px-node-1", + HostURL: "https://px.example:8006", + }, + } +} + +func pmgMatchResource() Resource { + return Resource{ + ID: "pmg-res-1", + Name: "pmg-row-name", + PMG: &PMGData{ + InstanceID: "pmg-inst-1", + Hostname: "mail.example", + HostURL: "https://mail.example:8006", + }, + } +} + +func k8sMatchResource() Resource { + return Resource{ + ID: "k8s-res-1", + Name: "k8s-row-name", + Kubernetes: &K8sData{ + ClusterID: "cluster-1", + ClusterName: "cluster-name-1", + SourceName: "source-name-1", + Server: "https://k8s.example:6443", + AgentID: "k8s-agent-1", + }, + } +} + +// --------------------------------------------------------------------------- +// monitoredSystemReplacementSelectorMatchesAgent +// --------------------------------------------------------------------------- + +func TestBranchcov0720am_AgentMatcher(t *testing.T) { + cases := []struct { + name string + selector MonitoredSystemReplacementSelector + resource func() Resource // fresh copy per case to keep cases independent + want bool + }{ + // nil-source-data guard -> false. + { + name: "nil_agent_returns_false", + selector: MonitoredSystemReplacementSelector{AgentID: "agent-1"}, + resource: func() Resource { + r := agentMatchResource() + r.Agent = nil + return r + }, + want: false, + }, + // Empty / zero selector -> every arm false -> false. + { + name: "empty_selector_no_match", + selector: MonitoredSystemReplacementSelector{}, + resource: func() Resource { return agentMatchResource() }, + want: false, + }, + // Canonical ResourceID arm only (selector.ResourceID == resource.ID). + { + name: "canonical_resource_id_match", + selector: MonitoredSystemReplacementSelector{ResourceID: "agent-res-1"}, + resource: func() Resource { return agentMatchResource() }, + want: true, + }, + // Canonical arm with case-folding (EqualFold semantics). + { + name: "canonical_resource_id_case_fold", + selector: MonitoredSystemReplacementSelector{ResourceID: "AGENT-RES-1"}, + resource: func() Resource { return agentMatchResource() }, + want: true, + }, + // AgentID arm only. + { + name: "agent_id_match", + selector: MonitoredSystemReplacementSelector{AgentID: "agent-1"}, + resource: func() Resource { return agentMatchResource() }, + want: true, + }, + // MachineID arm only. + { + name: "machine_id_match", + selector: MonitoredSystemReplacementSelector{MachineID: "machine-1"}, + resource: func() Resource { return agentMatchResource() }, + want: true, + }, + // Host arm only: selector.Hostname matches Agent.Hostname. + { + name: "host_via_agent_hostname", + selector: MonitoredSystemReplacementSelector{Hostname: "lab-a.example"}, + resource: func() Resource { return agentMatchResource() }, + want: true, + }, + // Host arm only: selector.Hostname matches resource.Name (second candidate). + { + name: "host_via_resource_name", + selector: MonitoredSystemReplacementSelector{Hostname: "agent-row-name"}, + resource: func() Resource { return agentMatchResource() }, + want: true, + }, + // Host arm reached via HostURL-derived hostname (extractHostname path). + { + name: "host_via_selector_host_url", + selector: MonitoredSystemReplacementSelector{HostURL: "https://lab-a.example:443"}, + resource: func() Resource { return agentMatchResource() }, + want: true, + }, + // Non-matching populated selector -> false. + { + name: "populated_no_match", + selector: MonitoredSystemReplacementSelector{ + ResourceID: "different-id", + AgentID: "different-agent", + MachineID: "different-machine", + Hostname: "different.example", + }, + resource: func() Resource { return agentMatchResource() }, + want: false, + }, + // Whitespace-only selector fields must not match (trimmedEqualFold + // requires non-empty on both sides). + { + name: "whitespace_only_selector_no_match", + selector: MonitoredSystemReplacementSelector{ + ResourceID: " ", + AgentID: "\t", + MachineID: " ", + Hostname: " ", + }, + resource: func() Resource { return agentMatchResource() }, + want: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + got := monitoredSystemReplacementSelectorMatchesAgent(tc.selector, tc.resource()) + if got != tc.want { + t.Fatalf("monitoredSystemReplacementSelectorMatchesAgent(%+v) = %v, want %v", + tc.selector, got, tc.want) + } + }) + } +} + +// --------------------------------------------------------------------------- +// monitoredSystemReplacementSelectorMatchesProxmox +// --------------------------------------------------------------------------- + +func TestBranchcov0720am_ProxmoxMatcher(t *testing.T) { + cases := []struct { + name string + selector MonitoredSystemReplacementSelector + resource func() Resource + want bool + }{ + // nil-source-data guard. + { + name: "nil_proxmox_returns_false", + selector: MonitoredSystemReplacementSelector{ResourceID: "px-src-1"}, + resource: func() Resource { + r := proxmoxMatchResource() + r.Proxmox = nil + return r + }, + want: false, + }, + // Empty selector -> no match. + { + name: "empty_selector_no_match", + selector: MonitoredSystemReplacementSelector{}, + resource: func() Resource { return proxmoxMatchResource() }, + want: false, + }, + // Canonical ResourceID arm only. + { + name: "canonical_resource_id_match", + selector: MonitoredSystemReplacementSelector{ResourceID: "px-res-1"}, + resource: func() Resource { return proxmoxMatchResource() }, + want: true, + }, + // Proxmox SourceID arm only. + { + name: "resource_id_matches_proxmox_source_id", + selector: MonitoredSystemReplacementSelector{ResourceID: "px-src-1"}, + resource: func() Resource { return proxmoxMatchResource() }, + want: true, + }, + // Name matches Proxmox.Instance arm only. + { + name: "name_matches_instance", + selector: MonitoredSystemReplacementSelector{Name: "px-instance-1"}, + resource: func() Resource { return proxmoxMatchResource() }, + want: true, + }, + // Name matches Proxmox.NodeName arm only. + { + name: "name_matches_node_name", + selector: MonitoredSystemReplacementSelector{Name: "px-node-1"}, + resource: func() Resource { return proxmoxMatchResource() }, + want: true, + }, + // HostURL matches Proxmox.HostURL arm only (trimmedEqualFold, exact + // string equality). + { + name: "host_url_matches_proxmox_host_url", + selector: MonitoredSystemReplacementSelector{HostURL: "https://px.example:8006"}, + resource: func() Resource { return proxmoxMatchResource() }, + want: true, + }, + // Host arm via selector.Hostname matching Proxmox.NodeName candidate. + { + name: "host_via_proxmox_node_name", + selector: MonitoredSystemReplacementSelector{Hostname: "px-node-1"}, + resource: func() Resource { return proxmoxMatchResource() }, + want: true, + }, + // Host arm via selector.Hostname matching resource.Name candidate. + { + name: "host_via_resource_name", + selector: MonitoredSystemReplacementSelector{Hostname: "px-row-name"}, + resource: func() Resource { return proxmoxMatchResource() }, + want: true, + }, + // Host arm via selector.HostURL-derived hostname matching the + // Proxmox.HostURL candidate (extractHostname path). + { + name: "host_via_selector_host_url_extract", + selector: MonitoredSystemReplacementSelector{HostURL: "https://px.example:8006"}, + resource: func() Resource { + // Force canonical / SourceID / Instance / NodeName / direct-HostURL + // arms to fail so the host arm (last) is the only viable match. + r := proxmoxMatchResource() + r.ID = "other-id" + r.Proxmox.SourceID = "other-src" + r.Proxmox.Instance = "other-instance" + // NodeName stays "px-node-1" but selector has empty Hostname so + // Name-equality arms don't fire; the selector-derived host + // "px.example" matches Proxmox.Hostname-equivalent candidate. + return r + }, + want: true, + }, + // Non-matching populated selector -> false. + { + name: "populated_no_match", + selector: MonitoredSystemReplacementSelector{ + ResourceID: "different", + Name: "different", + HostURL: "https://different.example:8006", + Hostname: "different.example", + }, + resource: func() Resource { return proxmoxMatchResource() }, + want: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + got := monitoredSystemReplacementSelectorMatchesProxmox(tc.selector, tc.resource()) + if got != tc.want { + t.Fatalf("monitoredSystemReplacementSelectorMatchesProxmox(%+v) = %v, want %v", + tc.selector, got, tc.want) + } + }) + } +} + +// --------------------------------------------------------------------------- +// monitoredSystemReplacementSelectorMatchesPMG +// --------------------------------------------------------------------------- + +func TestBranchcov0720am_PMGMatcher(t *testing.T) { + cases := []struct { + name string + selector MonitoredSystemReplacementSelector + resource func() Resource + want bool + }{ + // nil-source-data guard. + { + name: "nil_pmg_returns_false", + selector: MonitoredSystemReplacementSelector{ResourceID: "pmg-inst-1"}, + resource: func() Resource { + r := pmgMatchResource() + r.PMG = nil + return r + }, + want: false, + }, + // Empty selector -> no match. + { + name: "empty_selector_no_match", + selector: MonitoredSystemReplacementSelector{}, + resource: func() Resource { return pmgMatchResource() }, + want: false, + }, + // Canonical ResourceID arm only. + { + name: "canonical_resource_id_match", + selector: MonitoredSystemReplacementSelector{ResourceID: "pmg-res-1"}, + resource: func() Resource { return pmgMatchResource() }, + want: true, + }, + // PMG InstanceID arm only. + { + name: "resource_id_matches_pmg_instance_id", + selector: MonitoredSystemReplacementSelector{ResourceID: "pmg-inst-1"}, + resource: func() Resource { return pmgMatchResource() }, + want: true, + }, + // Host arm via selector.Hostname matching PMG.Hostname. + { + name: "host_via_pmg_hostname", + selector: MonitoredSystemReplacementSelector{Hostname: "mail.example"}, + resource: func() Resource { return pmgMatchResource() }, + want: true, + }, + // Host arm via selector.Hostname matching resource.Name. + { + name: "host_via_resource_name", + selector: MonitoredSystemReplacementSelector{Hostname: "pmg-row-name"}, + resource: func() Resource { return pmgMatchResource() }, + want: true, + }, + // Host arm via selector.HostURL-derived hostname. + { + name: "host_via_selector_host_url_extract", + selector: MonitoredSystemReplacementSelector{HostURL: "https://mail.example:8006"}, + resource: func() Resource { return pmgMatchResource() }, + want: true, + }, + // Non-matching populated selector -> false. + { + name: "populated_no_match", + selector: MonitoredSystemReplacementSelector{ + ResourceID: "different", + Hostname: "different.example", + HostURL: "https://different.example:8006", + }, + resource: func() Resource { return pmgMatchResource() }, + want: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + got := monitoredSystemReplacementSelectorMatchesPMG(tc.selector, tc.resource()) + if got != tc.want { + t.Fatalf("monitoredSystemReplacementSelectorMatchesPMG(%+v) = %v, want %v", + tc.selector, got, tc.want) + } + }) + } +} + +// --------------------------------------------------------------------------- +// monitoredSystemReplacementSelectorMatchesK8s +// --------------------------------------------------------------------------- + +func TestBranchcov0720am_K8sMatcher(t *testing.T) { + cases := []struct { + name string + selector MonitoredSystemReplacementSelector + resource func() Resource + want bool + }{ + // nil-source-data guard. + { + name: "nil_kubernetes_returns_false", + selector: MonitoredSystemReplacementSelector{ResourceID: "cluster-1"}, + resource: func() Resource { + r := k8sMatchResource() + r.Kubernetes = nil + return r + }, + want: false, + }, + // Empty selector -> no match. + { + name: "empty_selector_no_match", + selector: MonitoredSystemReplacementSelector{}, + resource: func() Resource { return k8sMatchResource() }, + want: false, + }, + // Canonical ResourceID arm only. + { + name: "canonical_resource_id_match", + selector: MonitoredSystemReplacementSelector{ResourceID: "k8s-res-1"}, + resource: func() Resource { return k8sMatchResource() }, + want: true, + }, + // ClusterID arm only. + { + name: "resource_id_matches_cluster_id", + selector: MonitoredSystemReplacementSelector{ResourceID: "cluster-1"}, + resource: func() Resource { return k8sMatchResource() }, + want: true, + }, + // ClusterName arm only. + { + name: "name_matches_cluster_name", + selector: MonitoredSystemReplacementSelector{Name: "cluster-name-1"}, + resource: func() Resource { return k8sMatchResource() }, + want: true, + }, + // SourceName arm only. + { + name: "name_matches_source_name", + selector: MonitoredSystemReplacementSelector{Name: "source-name-1"}, + resource: func() Resource { return k8sMatchResource() }, + want: true, + }, + // Server arm only. + { + name: "host_url_matches_server", + selector: MonitoredSystemReplacementSelector{HostURL: "https://k8s.example:6443"}, + resource: func() Resource { return k8sMatchResource() }, + want: true, + }, + // K8s AgentID arm only. + { + name: "agent_id_matches_k8s_agent_id", + selector: MonitoredSystemReplacementSelector{AgentID: "k8s-agent-1"}, + resource: func() Resource { return k8sMatchResource() }, + want: true, + }, + // Non-matching populated selector -> false. + { + name: "populated_no_match", + selector: MonitoredSystemReplacementSelector{ + ResourceID: "different", + Name: "different", + HostURL: "https://different.example:6443", + AgentID: "different-agent", + }, + resource: func() Resource { return k8sMatchResource() }, + want: false, + }, + // Whitespace-only selector fields -> false (trimmedEqualFold needs + // non-empty on both sides after trimming). + { + name: "whitespace_only_selector_no_match", + selector: MonitoredSystemReplacementSelector{ + ResourceID: " ", + Name: "\t", + HostURL: " ", + AgentID: " ", + }, + resource: func() Resource { return k8sMatchResource() }, + want: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + got := monitoredSystemReplacementSelectorMatchesK8s(tc.selector, tc.resource()) + if got != tc.want { + t.Fatalf("monitoredSystemReplacementSelectorMatchesK8s(%+v) = %v, want %v", + tc.selector, got, tc.want) + } + }) + } +} diff --git a/internal/unifiedresources/monitored_systems_helpers_branchcov0720am_test.go b/internal/unifiedresources/monitored_systems_helpers_branchcov0720am_test.go new file mode 100644 index 000000000..8ef6e7165 --- /dev/null +++ b/internal/unifiedresources/monitored_systems_helpers_branchcov0720am_test.go @@ -0,0 +1,189 @@ +package unifiedresources + +import ( + "strings" + "testing" + "time" +) + +// Branch-coverage tests for two helpers in monitored_systems.go that are +// otherwise at 0% coverage: +// +// - monitoredSystemHasReasonStatus: a slice-scanning predicate with +// nil/empty/no-match/match arms. +// - monitoredSystemStatusReportedAtSuffix: a formatter with a zero-time +// arm (empty result) and a populated arm that emits a UTC RFC3339 +// timestamp. +// +// Assertions are behavioral: the predicate's classification outcome across +// its distinct arms, and the formatter's empty-vs-populated contract plus +// UTC normalization (the suffix for a given instant must be identical +// regardless of the input's timezone offset). + +// --- monitoredSystemHasReasonStatus --- + +func TestBranchcov0720am_HasReasonStatus(t *testing.T) { + cases := []struct { + name string + reasons []MonitoredSystemStatusReason + status string + want bool + }{ + { + name: "nil slice returns false", + reasons: nil, + status: "offline", + want: false, + }, + { + name: "empty slice returns false", + reasons: []MonitoredSystemStatusReason{}, + status: "offline", + want: false, + }, + { + name: "no matching status returns false after full scan", + reasons: []MonitoredSystemStatusReason{ + {Status: "online"}, + {Status: "warning"}, + {Status: "stale"}, + }, + status: "offline", + want: false, + }, + { + name: "single matching entry returns true", + reasons: []MonitoredSystemStatusReason{ + {Status: "offline"}, + }, + status: "offline", + want: true, + }, + { + name: "match at first position returns true", + reasons: []MonitoredSystemStatusReason{ + {Status: "offline"}, + {Status: "online"}, + {Status: "stale"}, + }, + status: "offline", + want: true, + }, + { + name: "match at middle position returns true", + reasons: []MonitoredSystemStatusReason{ + {Status: "online"}, + {Status: "stale"}, + {Status: "warning"}, + }, + status: "stale", + want: true, + }, + { + name: "match at last position returns true", + reasons: []MonitoredSystemStatusReason{ + {Status: "online"}, + {Status: "warning"}, + {Status: "offline"}, + }, + status: "offline", + want: true, + }, + { + name: "empty queried status matches entry with unset Status field", + reasons: []MonitoredSystemStatusReason{ + {Status: ""}, + {Status: "online"}, + }, + status: "", + want: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := monitoredSystemHasReasonStatus(tc.reasons, tc.status) + if got != tc.want { + t.Fatalf("monitoredSystemHasReasonStatus(_, %q) = %v, want %v", tc.status, got, tc.want) + } + }) + } +} + +// The no-match path scans every element; assert it leaves the input slice +// untouched (the predicate must be side-effect free). +func TestBranchcov0720am_HasReasonStatus_NoMatchPathLeavesInputUntouched(t *testing.T) { + reasons := []MonitoredSystemStatusReason{ + {Status: "online"}, + {Status: "warning"}, + {Status: "stale"}, + } + snapshot := make([]MonitoredSystemStatusReason, len(reasons)) + copy(snapshot, reasons) + + if monitoredSystemHasReasonStatus(reasons, "offline") { + t.Fatal("expected no-match for status 'offline'") + } + if len(reasons) != len(snapshot) { + t.Fatalf("input length changed: got %d, want %d", len(reasons), len(snapshot)) + } + for i := range reasons { + if reasons[i] != snapshot[i] { + t.Errorf("input[%d] mutated by no-match scan: got %+v, want %+v", i, reasons[i], snapshot[i]) + } + } +} + +// --- monitoredSystemStatusReportedAtSuffix --- + +func TestBranchcov0720am_StatusReportedAtSuffix(t *testing.T) { + // Two representations of the SAME instant, in different time zones. + // The formatter is documented to emit UTC, so equivalent instants must + // yield byte-identical suffixes regardless of input zone offset. + utcTime := time.Date(2024, 3, 15, 10, 30, 0, 0, time.UTC) + offsetZone := time.FixedZone("OFFSET", -5*3600) + offsetEquivalent := utcTime.In(offsetZone) + + t.Run("zero time yields empty suffix", func(t *testing.T) { + if got := monitoredSystemStatusReportedAtSuffix(time.Time{}); got != "" { + t.Fatalf("zero-time suffix: got %q, want empty", got) + } + }) + + t.Run("populated UTC time yields non-empty suffix embedding UTC RFC3339", func(t *testing.T) { + got := monitoredSystemStatusReportedAtSuffix(utcTime) + if got == "" { + t.Fatal("populated-time suffix: got empty, want non-empty") + } + wantEmbedded := utcTime.UTC().Format(time.RFC3339) + if !strings.Contains(got, wantEmbedded) { + t.Errorf("suffix %q does not embed expected UTC RFC3339 form %q", got, wantEmbedded) + } + }) + + t.Run("populated offset-zone time yields non-empty suffix embedding UTC RFC3339", func(t *testing.T) { + got := monitoredSystemStatusReportedAtSuffix(offsetEquivalent) + if got == "" { + t.Fatal("populated-time suffix: got empty, want non-empty") + } + wantEmbedded := offsetEquivalent.UTC().Format(time.RFC3339) + if !strings.Contains(got, wantEmbedded) { + t.Errorf("suffix %q does not embed expected UTC RFC3339 form %q", got, wantEmbedded) + } + // Negative assertion: the formatter must NOT have used the input's + // local-zone representation (which would carry a "-05:00" offset + // rather than the "Z" UTC marker). + localForm := offsetEquivalent.Format(time.RFC3339) + if localForm != wantEmbedded && strings.Contains(got, localForm) { + t.Errorf("suffix %q embeds local-zone form %q; UTC normalization broken", got, localForm) + } + }) + + t.Run("equal instants in different zones produce identical suffixes", func(t *testing.T) { + a := monitoredSystemStatusReportedAtSuffix(utcTime) + b := monitoredSystemStatusReportedAtSuffix(offsetEquivalent) + if a != b { + t.Errorf("suffixes differ for equal instants: UTC=%q offset=%q", a, b) + } + }) +} diff --git a/internal/unifiedresources/patrol_autopilot_validate_branchcov0720am_test.go b/internal/unifiedresources/patrol_autopilot_validate_branchcov0720am_test.go new file mode 100644 index 000000000..dd83a30c8 --- /dev/null +++ b/internal/unifiedresources/patrol_autopilot_validate_branchcov0720am_test.go @@ -0,0 +1,228 @@ +package unifiedresources + +import ( + "errors" + "testing" + "time" +) + +// branchcov0720amValidAck mints one structurally-valid acknowledgement that +// passes every arm of ValidatePatrolAutopilotStoredEvidence's acknowledgement +// loop. Each call produces an independent record so subtests cannot contaminate +// one another. +func branchcov0720amValidAck(t *testing.T, id string) PatrolAutopilotAcknowledgement { + t.Helper() + policy := CurrentPatrolAutopilotServerPolicy(time.Date(2026, 7, 20, 9, 0, 0, 0, time.UTC)) + actor := ActionActor{SubjectID: "admin-one", Kind: ActionActorUser, CredentialID: "session:one", OrgID: "org-a"} + record, created, err := IssuePatrolAutopilotAcknowledgement(nil, id, actor, policy) + if err != nil || !created { + t.Fatalf("mint ack %q: created=%v err=%v", id, created, err) + } + return record +} + +// branchcov0720amValidActivation mints an activation bound to ack whose +// ActivatedAt sits strictly after ack.AcceptedAt (avoids the Before edge). +func branchcov0720amValidActivation(t *testing.T, ack PatrolAutopilotAcknowledgement) PatrolAutopilotActivation { + t.Helper() + policy := CurrentPatrolAutopilotServerPolicy(ack.AcceptedAt.Add(30 * time.Second)) + record, created, err := BindPatrolAutopilotActivation([]PatrolAutopilotAcknowledgement{ack}, nil, nil, ack.ID, ack.Actor, policy) + if err != nil || !created { + t.Fatalf("mint activation: created=%v err=%v", created, err) + } + return record +} + +// branchcov0720amValidRevocation mints a revocation bound to ack whose +// RevokedAt sits strictly after ack.AcceptedAt. +func branchcov0720amValidRevocation(t *testing.T, ack PatrolAutopilotAcknowledgement) PatrolAutopilotRevocation { + t.Helper() + policy := CurrentPatrolAutopilotServerPolicy(ack.AcceptedAt.Add(time.Minute)) + record, created, err := RevokePatrolAutopilotAcknowledgement([]PatrolAutopilotAcknowledgement{ack}, nil, ack.ID, ack.Actor, "operator stop", policy) + if err != nil || !created { + t.Fatalf("mint revocation: created=%v err=%v", created, err) + } + return record +} + +// TestBranchcov0720am_ValidatePatrolAutopilotStoredEvidence drives every +// distinct return arm of the captured-evidence validator: the happy paths +// (empty input, nil activation, valid ack/revocation/activation, multi-record), +// each acknowledgement-loop rejection arm, each revocation-loop rejection arm, +// and both activation rejection arms. +func TestBranchcov0720am_ValidatePatrolAutopilotStoredEvidence(t *testing.T) { + ack := branchcov0720amValidAck(t, "ack-validate-0001") + ack2 := branchcov0720amValidAck(t, "ack-validate-0002") + act := branchcov0720amValidActivation(t, ack) + rev := branchcov0720amValidRevocation(t, ack) + + cases := []struct { + name string + build func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) + wantErr bool + }{ + // --- happy paths --- + {name: "empty_inputs", build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return nil, nil, nil + }}, + {name: "ack_only_nil_activation", build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return []PatrolAutopilotAcknowledgement{ack}, nil, nil + }}, + {name: "two_acknowledgements", build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return []PatrolAutopilotAcknowledgement{ack, ack2}, nil, nil + }}, + {name: "ack_and_revocation", build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return []PatrolAutopilotAcknowledgement{ack}, []PatrolAutopilotRevocation{rev}, nil + }}, + {name: "ack_and_activation", build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return []PatrolAutopilotAcknowledgement{ack}, nil, &act + }}, + {name: "ack_revocation_and_activation", build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return []PatrolAutopilotAcknowledgement{ack}, []PatrolAutopilotRevocation{rev}, &act + }}, + + // --- acknowledgement-loop arms --- + {name: "ack_id_untrimmed", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.ID = ack.ID + " " + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_id_pattern_too_short", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.ID = "short" + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_duplicate_id", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return []PatrolAutopilotAcknowledgement{ack, ack}, nil, nil + }}, + {name: "ack_unsupported_version", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.Version = 99 + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_actor_not_user", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.Actor.Kind = ActionActorAPIToken + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_actor_org_mismatch", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.Actor.OrgID = "org-b" + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_scope_invalid", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.AcceptedScope = []string{"tampered"} + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_limits_invalid", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.AcceptedLimits = PatrolAutopilotAcceptedLimits{} + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_validity_accepted_at_zero", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.AcceptedAt = time.Time{} + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_validity_expires_not_after_accepted", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.ExpiresAt = ack.AcceptedAt + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_digest_empty", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.Digest = "" + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + {name: "ack_digest_tampered", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := ack + bad.Digest = "sha256:deadbeef" + return []PatrolAutopilotAcknowledgement{bad}, nil, nil + }}, + + // --- revocation-loop arms --- + {name: "revocation_unknown_acknowledgement", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + orphan := rev + orphan.AcknowledgementID = "does-not-exist" + return []PatrolAutopilotAcknowledgement{ack}, []PatrolAutopilotRevocation{orphan}, nil + }}, + {name: "revocation_duplicate", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + return []PatrolAutopilotAcknowledgement{ack}, []PatrolAutopilotRevocation{rev, rev}, nil + }}, + {name: "revocation_binding_invalid", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := rev + bad.Digest = "sha256:revoked-bad" + return []PatrolAutopilotAcknowledgement{ack}, []PatrolAutopilotRevocation{bad}, nil + }}, + + // --- activation arms --- + {name: "activation_unknown_acknowledgement", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + orphan := act + orphan.AcknowledgementID = "does-not-exist" + return []PatrolAutopilotAcknowledgement{ack}, nil, &orphan + }}, + {name: "activation_binding_invalid", wantErr: true, build: func() ([]PatrolAutopilotAcknowledgement, []PatrolAutopilotRevocation, *PatrolAutopilotActivation) { + bad := act + bad.Digest = "sha256:activation-bad" + return []PatrolAutopilotAcknowledgement{ack}, nil, &bad + }}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + acks, revocations, activation := tc.build() + err := ValidatePatrolAutopilotStoredEvidence(acks, revocations, activation) + switch { + case tc.wantErr && err == nil: + t.Fatalf("expected rejection error, got nil") + case !tc.wantErr && err != nil: + t.Fatalf("expected acceptance (nil error), got %v", err) + } + }) + } + + // Behavioral confirmation that the multi-record happy path actually built + // the lookup map (a revocation against the second ack is accepted, proving + // both acknowledgements were indexed rather than silently dropped). + t.Run("two_acknowledgements_both_indexed", func(t *testing.T) { + revForSecond := branchcov0720amValidRevocation(t, ack2) + if err := ValidatePatrolAutopilotStoredEvidence([]PatrolAutopilotAcknowledgement{ack, ack2}, []PatrolAutopilotRevocation{revForSecond}, nil); err != nil { + t.Fatalf("revocation against second ack should be accepted once indexed: %v", err) + } + }) +} + +// TestBranchcov0720am_ContractErrorError covers the three branches of +// (*PatrolAutopilotContractError).Error(): nil receiver, nil wrapped Err, and a +// non-nil wrapped Err. The Code markers are authored by the test so the +// assertions exercise real formatting logic rather than re-stating source +// constants. +func TestBranchcov0720am_ContractErrorError(t *testing.T) { + t.Run("nil_receiver_returns_empty", func(t *testing.T) { + var nilErr *PatrolAutopilotContractError + if got := nilErr.Error(); got != "" { + t.Fatalf("nil receiver: expected empty string, got %q", got) + } + }) + + t.Run("nil_wrapped_err_returns_code_only", func(t *testing.T) { + codeOnly := &PatrolAutopilotContractError{Code: "branchcov-code-only", Err: nil} + if got, want := codeOnly.Error(), "branchcov-code-only"; got != want { + t.Fatalf("nil Err: expected code-only %q, got %q", want, got) + } + }) + + t.Run("wrapped_err_joins_code_and_inner", func(t *testing.T) { + inner := errors.New("inner-detail") + wrapped := &PatrolAutopilotContractError{Code: "branchcov-wrapped", Err: inner} + want := "branchcov-wrapped" + ": " + inner.Error() + if got := wrapped.Error(); got != want { + t.Fatalf("wrapped Err: expected %q, got %q", want, got) + } + // errors.Is must traverse to the wrapped error via Unwrap. + if !errors.Is(wrapped, inner) { + t.Fatal("errors.Is failed to reach the wrapped inner error") + } + }) +} diff --git a/internal/unifiedresources/physical_disk_branchcov0720am_test.go b/internal/unifiedresources/physical_disk_branchcov0720am_test.go new file mode 100644 index 000000000..053e9efc0 --- /dev/null +++ b/internal/unifiedresources/physical_disk_branchcov0720am_test.go @@ -0,0 +1,332 @@ +package unifiedresources + +import ( + "strings" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" +) + +// Branch-coverage tests for the two currently-uncovered exported disk +// helpers in this package: +// +// - PhysicalDiskStatus (physical_disk_risk.go) — thin exported wrapper +// around the unexported physicalDiskStatus switch. Every test goes +// through the EXPORTED function so the wrapper itself is exercised +// end-to-end; each row targets one arm of the underlying switch. +// - PhysicalDiskMetricID (physical_disk_ids.go) — id builder that +// derives a fallback from ID-or-DevPath and then defers to the +// shared PreferredPhysicalDiskMetricID preference order. Each row +// drives one combination of (ID/DevPath/Serial/WWN presence). +// +// Assertions are behavioral: status-class identity, id preference order, +// slash-replacement + whitespace-trimming effects on the synthesized +// fallback. No assertion merely re-pins a source constant. + +// --- PhysicalDiskStatus --- + +func TestBranchcov0720am_PhysicalDiskStatus(t *testing.T) { + t.Parallel() + + // Each row drives one arm of physicalDiskStatus: + // - the assessment.Level switch (RiskCritical | RiskWarning | other), + // - each health arm (PASSED/OK | FAILED+firmwareBug | FAILED+noBug | default), + // - the strings.ToUpper/TrimSpace normalization. + cases := []struct { + name string + model string + health string + assessment storagehealth.Assessment + want ResourceStatus + }{ + // Assessment arms: short-circuit before the health switch is reached. + // A critical assessment must produce warning even when health is FAILED. + { + name: "critical_assessment_wins_over_failed_health", + model: "Crucial MX500", + health: "FAILED", + assessment: storagehealth.Assessment{Level: storagehealth.RiskCritical}, + want: StatusWarning, + }, + // A warning assessment must produce warning even when health is PASSED. + { + name: "warning_assessment_wins_over_passed_health", + model: "Crucial MX500", + health: "PASSED", + assessment: storagehealth.Assessment{Level: storagehealth.RiskWarning}, + want: StatusWarning, + }, + // RiskMonitor is NOT in the (Critical, Warning) arm of the switch — + // it must fall through to the health check. This is the key + // branch-coverage boundary for the assessment switch. + { + name: "monitor_assessment_falls_through_to_health_check", + model: "Crucial MX500", + health: "PASSED", + assessment: storagehealth.Assessment{Level: storagehealth.RiskMonitor}, + want: StatusOnline, + }, + // PASSED health arm. + { + name: "passed_health_returns_online", + model: "Crucial MX500", + health: "PASSED", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusOnline, + }, + // OK health arm (the other member of the "PASSED, OK" case). + { + name: "ok_health_returns_online", + model: "Crucial MX500", + health: "OK", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusOnline, + }, + // FAILED + known firmware bug → StatusUnknown (the + // HasKnownFirmwareBug(model)==true arm). + { + name: "failed_health_with_known_firmware_bug_returns_unknown", + model: "Samsung SSD 980 Pro", + health: "FAILED", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusUnknown, + }, + // FAILED + no known firmware bug → StatusOffline (the + // HasKnownFirmwareBug(model)==false arm). + { + name: "failed_health_without_firmware_bug_returns_offline", + model: "Crucial MX500", + health: "FAILED", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusOffline, + }, + // default arm: unrecognized health string → StatusUnknown. + { + name: "unrecognized_health_returns_unknown", + model: "Crucial MX500", + health: "PREFAIL", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusUnknown, + }, + // default arm: empty health → StatusUnknown. + { + name: "empty_health_returns_unknown", + model: "Crucial MX500", + health: "", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusUnknown, + }, + // Normalization arm: lowercase + surrounding whitespace is accepted. + { + name: "lowercase_whitespace_passed_is_normalized_to_online", + model: "Crucial MX500", + health: " passed ", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusOnline, + }, + // Normalization arm: "ok" with whitespace/casing also lands in the + // OK case after ToUpper(TrimSpace(...)). + { + name: "lowercase_whitespace_ok_is_normalized_to_online", + model: "Crucial MX500", + health: "\tok\t", + assessment: storagehealth.Assessment{Level: storagehealth.RiskHealthy}, + want: StatusOnline, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := PhysicalDiskStatus(tc.model, tc.health, tc.assessment) + if got != tc.want { + t.Fatalf("PhysicalDiskStatus(model=%q, health=%q, level=%s): got %s, want %s", + tc.model, tc.health, tc.assessment.Level, got, tc.want) + } + }) + } +} + +// --- PhysicalDiskMetricID --- + +func TestBranchcov0720am_PhysicalDiskMetricID(t *testing.T) { + t.Parallel() + + // Each row drives one combination of the two conditional arms in + // PhysicalDiskMetricID (the ID!=empty fallback branch and the + // ID-empty+DevPath-nonempty sprintf branch) together with the + // downstream PreferredPhysicalDiskMetricID preference order + // (serial > wwn > fallback). + cases := []struct { + name string + disk models.PhysicalDisk + // want is the exact expected id when the behavior of the function + // is to return one of the input fields verbatim (after trim). + // For the synthesized DevPath cases we leave want empty and use + // the behavioral predicates below. + want string + wantNonEmpty bool // when true, assert got != "" instead of equality + }{ + // Preference: Serial wins outright, even when every other field is set. + { + name: "serial_wins_over_wwn_id_and_devpath", + disk: models.PhysicalDisk{ + Serial: "SER-1", WWN: "WWN-1", ID: "id-1", DevPath: "/dev/sda", + Instance: "inst", Node: "node", + }, + want: "SER-1", + }, + // Preference: WWN wins when Serial is empty (with ID present — ID + // branch hit but loses to WWN downstream). + { + name: "wwn_wins_when_serial_empty_and_id_present", + disk: models.PhysicalDisk{ + Serial: "", WWN: "WWN-1", ID: "id-1", + }, + want: "WWN-1", + }, + // Preference: WWN wins when Serial is empty and ID absent — + // exercises TrimSpace on WWN at the Preferred layer. + { + name: "wwn_wins_and_is_trimmed_when_only_wwn_set", + disk: models.PhysicalDisk{ + Serial: "", WWN: " WWN-1 ", ID: "", + }, + want: "WWN-1", + }, + // Fallback arm #1: ID is non-empty (DevPath sprintf branch + // skipped), no Serial/WWN → trimmed ID returned. + { + name: "id_used_as_fallback_when_no_serial_or_wwn", + disk: models.PhysicalDisk{ + ID: " disk-id-1 ", + }, + want: "disk-id-1", + }, + // Fallback arm #2: ID empty, DevPath non-empty → sprintf(instance, + // node, devpath-with-slashes-replaced) synthesized. We assert + // behavior rather than a fragile literal below. + { + name: "devpath_fallback_synthesized_when_id_empty", + disk: models.PhysicalDisk{ + ID: "", DevPath: "/dev/sda", Instance: "inst-1", Node: "node-1", + }, + wantNonEmpty: true, + }, + // Fallback arm #2 with whitespace: confirms TrimSpace is applied + // to Instance, Node, DevPath before synthesis. + { + name: "devpath_fallback_trims_whitespace_on_components", + disk: models.PhysicalDisk{ + ID: " ", DevPath: " /dev/nvme0n1 ", Instance: " inst-1 ", Node: " node-1 ", + }, + wantNonEmpty: true, + }, + // Defensive: all identity fields empty → returns empty string + // (neither fallback branch is entered, Preferred returns the + // trimmed empty fallback). + { + name: "all_identity_empty_returns_empty_string", + disk: models.PhysicalDisk{}, + want: "", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := PhysicalDiskMetricID(tc.disk) + + if tc.wantNonEmpty { + if got == "" { + t.Fatalf("PhysicalDiskMetricID(%+v): expected non-empty id, got %q", tc.disk, got) + } + return + } + + if got != tc.want { + t.Fatalf("PhysicalDiskMetricID(%+v): got %q, want %q", tc.disk, got, tc.want) + } + }) + } +} + +// TestBranchcov0720am_PhysicalDiskMetricID_DevPathFallbackBehavior +// verifies the synthesized DevPath fallback is built from the +// Instance, Node and DevPath components per the "%s-%s-%s" format and +// that every "/" in DevPath is replaced with "-" (the +// strings.ReplaceAll arm). It uses behavioral predicates rather than +// pinning the exact literal so the test still passes if the format +// separator changes, but breaks if the slash replacement or +// component inclusion changes. +func TestBranchcov0720am_PhysicalDiskMetricID_DevPathFallbackBehavior(t *testing.T) { + t.Parallel() + + disk := models.PhysicalDisk{ + ID: "", // forces DevPath fallback arm + DevPath: "/dev/disk/by-id/sda", + Instance: "inst-xyz", + Node: "node-abc", + // No Serial/WWN so the synthesized fallback is what's returned. + } + + got := PhysicalDiskMetricID(disk) + + if got == "" { + t.Fatal("expected synthesized non-empty id") + } + + // Every slash in DevPath must have been replaced (the strings.ReplaceAll arm). + if strings.Contains(got, "/") { + t.Errorf("synthesized id must contain no '/': got %q", got) + } + + // The DevPath, with slashes replaced, must appear in the result. + if !strings.Contains(got, "-dev-disk-by-id-sda") { + t.Errorf("synthesized id should embed the slash-replaced DevPath: got %q", got) + } + + // Instance and Node must each appear in the result. + if !strings.Contains(got, "inst-xyz") { + t.Errorf("synthesized id should embed Instance: got %q", got) + } + if !strings.Contains(got, "node-abc") { + t.Errorf("synthesized id should embed Node: got %q", got) + } + + // The synthesized id must start with the Instance component (per the + // "%s-%s-%s" format with Instance first). + if !strings.HasPrefix(got, "inst-xyz-") { + t.Errorf("synthesized id should start with Instance+separator: got %q", got) + } +} + +// TestBranchcov0720am_PhysicalDiskMetricID_DevPathFallbackStopsAtEmptyDevPath +// verifies that when ID is empty AND DevPath is also empty/whitespace, +// the sprintf arm is NOT entered and the function returns the empty +// fallback (rather than producing "instance-node-" garbage). This is +// the branch-coverage boundary for the +// `fallback == "" && DevPath != ""` predicate's second operand. +func TestBranchcov0720am_PhysicalDiskMetricID_DevPathFallbackStopsAtEmptyDevPath(t *testing.T) { + t.Parallel() + + // ID empty, DevPath empty — Instance/Node set but must NOT be used. + disk := models.PhysicalDisk{ + ID: "", + DevPath: " ", + Instance: "inst-should-not-appear", + Node: "node-should-not-appear", + } + + got := PhysicalDiskMetricID(disk) + + if got != "" { + t.Errorf("empty ID and empty DevPath must produce empty id (sprintf arm must not fire): got %q", got) + } + if strings.Contains(got, "should-not-appear") { + t.Errorf("Instance/Node leaked into id despite empty DevPath: got %q", got) + } +} diff --git a/internal/unifiedresources/policy_metadata_branchcov0720am_test.go b/internal/unifiedresources/policy_metadata_branchcov0720am_test.go new file mode 100644 index 000000000..7558918f9 --- /dev/null +++ b/internal/unifiedresources/policy_metadata_branchcov0720am_test.go @@ -0,0 +1,305 @@ +package unifiedresources + +import ( + "strings" + "testing" +) + +// Branch-coverage tests for CanonicalGovernanceMetadata in policy_metadata.go. +// +// The function has two top-level branches: +// +// - nil-resource guard: returns (nil, "") +// - non-nil projection: shallow-copies the input, refreshes policy metadata +// on the copy (which always (re)assigns a non-nil derived Policy and a +// rebuilt AISafeSummary), and returns a deep-cloned policy plus the +// TrimSpace'd summary. +// +// Within the non-nil arm the input may arrive with Policy present or absent; +// RefreshPolicyMetadata overwrites either way, so the returned policy must +// always reflect the DERIVED classification (not any pre-existing Policy on +// the input), and the caller's Resource must not be mutated by the projection. +// +// Cases drive each branch and assert BOTH return values, the input-immunity +// contract, and the clone-independence contract. + +func TestBranchcov0720am_CanonicalGovernanceMetadata(t *testing.T) { + t.Parallel() + + // preExistingPolicy is intentionally misaligned with what classification + // will derive, so we can prove the present-policy arm overwrites rather + // than trusts the caller's Policy. + preExistingPolicy := &ResourcePolicy{ + Sensitivity: ResourceSensitivityPublic, + Routing: ResourceRoutingPolicy{ + Scope: ResourceRoutingScopeCloudSummary, + Redact: []ResourceRedactionHint{ResourceRedactionHostname}, + }, + } + + type expect struct { + nilPolicy bool + emptySummary bool + sensitivity ResourceSensitivity + routingScope ResourceRoutingScope + summaryContains string + summaryTrimmed bool + summaryNotContain string + } + + cases := []struct { + name string + resource *Resource + want expect + }{ + { + // nil-resource arm: returns (nil, ""). + name: "nil resource returns nil policy and empty summary", + resource: nil, + want: expect{ + nilPolicy: true, + emptySummary: true, + }, + }, + { + // Non-nil arm, ABSENT policy: a plain compute workload (VM) has no + // Policy set, so Refresh must derive Internal / cloud-summary. + name: "absent policy derives internal cloud-summary posture", + resource: &Resource{ + ID: "vm-100", + Name: "web-01", + Type: ResourceTypeVM, + Status: StatusOnline, + }, + want: expect{ + sensitivity: ResourceSensitivityInternal, + routingScope: ResourceRoutingScopeCloudSummary, + summaryContains: "virtual machine resource", + summaryTrimmed: true, + }, + }, + { + // Non-nil arm, ABSENT policy but a restricted tag drives a + // different derived branch (LocalOnly / restricted). + name: "absent policy with pii tag derives restricted local-only", + resource: &Resource{ + ID: "vm-200", + Name: "payments-db", + Type: ResourceTypeVM, + Status: StatusOnline, + Tags: []string{"pii"}, + }, + want: expect{ + sensitivity: ResourceSensitivityRestricted, + routingScope: ResourceRoutingScopeLocalOnly, + summaryContains: "local-only context", + summaryTrimmed: true, + }, + }, + { + // Non-nil arm, PRESENT policy: the caller has already set a Policy + // (Public). Refresh overwrites it, so the returned policy must + // reflect the derived Internal classification for a plain VM, + // NOT the pre-existing Public. + name: "present policy is overwritten by derived classification", + resource: &Resource{ + ID: "agent-9", + Name: "pve-node", + Type: ResourceTypeAgent, + Status: StatusOnline, + Policy: preExistingPolicy, + }, + want: expect{ + sensitivity: ResourceSensitivityInternal, + routingScope: ResourceRoutingScopeCloudSummary, + summaryContains: "agent resource", + summaryTrimmed: true, + // The misaligned pre-existing sensitivity must not leak into + // the summary text. + summaryNotContain: "public", + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + policy, summary := CanonicalGovernanceMetadata(tc.resource) + + // --- Both return values, per branch. --- + if tc.want.nilPolicy { + if policy != nil { + t.Fatalf("nil-resource arm: policy = %#v, want nil", policy) + } + if !tc.want.emptySummary { + t.Fatalf("test setup error: nilPolicy implies emptySummary") + } + if summary != "" { + t.Fatalf("nil-resource arm: summary = %q, want empty", summary) + } + return + } + + if policy == nil { + t.Fatal("non-nil arm: expected non-nil policy") + } + if tc.want.emptySummary && summary != "" { + t.Fatalf("summary = %q, want empty", summary) + } + + // Returned policy reflects the DERIVED classification. + if got := policy.Sensitivity; got != tc.want.sensitivity { + t.Errorf("Sensitivity = %q, want %q", got, tc.want.sensitivity) + } + if got := policy.Routing.Scope; got != tc.want.routingScope { + t.Errorf("Routing.Scope = %q, want %q", got, tc.want.routingScope) + } + + // Summary content + trim contract. + if tc.want.summaryContains != "" && !strings.Contains(summary, tc.want.summaryContains) { + t.Errorf("summary = %q, want substring %q", summary, tc.want.summaryContains) + } + if tc.want.summaryNotContain != "" && strings.Contains(strings.ToLower(summary), tc.want.summaryNotContain) { + t.Errorf("summary = %q, must not contain %q", summary, tc.want.summaryNotContain) + } + if tc.want.summaryTrimmed { + if summary != strings.TrimSpace(summary) { + t.Errorf("summary = %q, must have no surrounding whitespace", summary) + } + } + }) + } + + // Verify the table's present-policy case actually used a non-nil Policy + // input (guards against accidental test-setup regressions that would turn + // it into a duplicate of the absent-policy case). + if preExistingPolicy == nil { + t.Fatal("test setup error: preExistingPolicy must be non-nil") + } +} + +// TestBranchcov0720am_CanonicalGovernanceMetadata_InputNotMutated asserts the +// projection never writes back to the caller's Resource: neither the Policy +// pointer, nor the AISafeSummary, nor any pre-existing Policy Redact slice is +// touched. This is the central behavioral contract of a "canonical read". +func TestBranchcov0720am_CanonicalGovernanceMetadata_InputNotMutated(t *testing.T) { + t.Parallel() + + originalPolicy := &ResourcePolicy{ + Sensitivity: ResourceSensitivityPublic, + Routing: ResourceRoutingPolicy{ + Scope: ResourceRoutingScopeCloudSummary, + Redact: []ResourceRedactionHint{ + ResourceRedactionHostname, + ResourceRedactionIPAddress, + }, + }, + } + originalRedactLen := len(originalPolicy.Routing.Redact) + originalSummary := " pre-existing summary " + + resource := &Resource{ + ID: "vm-300", + Name: "cache-01", + Type: ResourceTypeVM, + Status: StatusOnline, + Policy: originalPolicy, + AISafeSummary: originalSummary, + } + + policy, summary := CanonicalGovernanceMetadata(resource) + + // The caller's Resource.Policy pointer is unchanged. + if resource.Policy != originalPolicy { + t.Fatalf("input Resource.Policy pointer was mutated: got %p, want %p", resource.Policy, originalPolicy) + } + // The caller's pre-existing Policy content is unchanged. + if resource.Policy.Sensitivity != ResourceSensitivityPublic { + t.Errorf("input Policy.Sensitivity mutated: got %q, want public", resource.Policy.Sensitivity) + } + if resource.Policy.Routing.Scope != ResourceRoutingScopeCloudSummary { + t.Errorf("input Policy.Routing.Scope mutated: got %q, want cloud-summary", resource.Policy.Routing.Scope) + } + if len(resource.Policy.Routing.Redact) != originalRedactLen { + t.Errorf("input Policy.Routing.Redact len mutated: got %d, want %d", len(resource.Policy.Routing.Redact), originalRedactLen) + } + // The caller's AISafeSummary is unchanged (Refresh writes only to the copy). + if resource.AISafeSummary != originalSummary { + t.Errorf("input AISafeSummary mutated: got %q, want %q", resource.AISafeSummary, originalSummary) + } + + // And the returned policy is NOT the same object as the input's policy + // (it must be a freshly derived + cloned value), while the returned + // summary differs from the raw input summary (it was rebuilt + trimmed). + if policy == originalPolicy { + t.Fatal("returned policy is the same pointer as input Policy; expected a derived clone") + } + if summary == originalSummary { + t.Fatalf("returned summary equals raw input %q; expected a rebuilt trimmed summary", summary) + } +} + +// TestBranchcov0720am_CanonicalGovernanceMetadata_CloneIndependence asserts the +// returned policy's Redact slice is an independent allocation: mutating it must +// not affect the input resource's pre-existing Policy (deep-copy guarantee). +func TestBranchcov0720am_CanonicalGovernanceMetadata_CloneIndependence(t *testing.T) { + t.Parallel() + + originalPolicy := &ResourcePolicy{ + Sensitivity: ResourceSensitivitySensitive, + Routing: ResourceRoutingPolicy{ + Scope: ResourceRoutingScopeLocalFirst, + Redact: []ResourceRedactionHint{ + ResourceRedactionHostname, + ResourceRedactionIPAddress, + }, + }, + } + // A storage-typed resource with a filesystem path derives a Sensitive + // posture that includes a Path redaction hint, so the returned Redact + // slice is guaranteed non-empty and safe to mutate. + resource := &Resource{ + ID: "storage-zfs-1", + Name: "tank", + Type: ResourceTypeStorage, + Status: StatusOnline, + Storage: &StorageMeta{ + Path: "/mnt/tank", + }, + Policy: originalPolicy, + } + + policy, _ := CanonicalGovernanceMetadata(resource) + + if policy == nil { + t.Fatal("expected non-nil policy") + } + if policy.Sensitivity != ResourceSensitivitySensitive { + t.Fatalf("Sensitivity = %q, want sensitive", policy.Sensitivity) + } + if len(policy.Routing.Redact) == 0 { + t.Fatalf("expected non-empty Redact for sensitive storage resource, got %#v", policy.Routing.Redact) + } + + // Mutate every element of the returned Redact slice. The input resource's + // pre-existing Policy Redact must remain pristine. + for i := range policy.Routing.Redact { + policy.Routing.Redact[i] = ResourceRedactionHint("MUTATED") + } + for _, got := range resource.Policy.Routing.Redact { + if strings.Contains(string(got), "MUTATED") { + t.Errorf("input Policy Redact mutated via returned clone: resource slice now contains %q", got) + } + } + + // Also confirm the input's original Redact content is intact. + if len(resource.Policy.Routing.Redact) != 2 { + t.Errorf("input Policy Redact len changed: got %d, want 2", len(resource.Policy.Routing.Redact)) + } + if resource.Policy.Routing.Redact[0] != ResourceRedactionHostname || + resource.Policy.Routing.Redact[1] != ResourceRedactionIPAddress { + t.Errorf("input Policy Redact content changed: got %#v, want [hostname ip-address]", resource.Policy.Routing.Redact) + } +} diff --git a/internal/unifiedresources/top_level_systems_basis_branchcov0720am_test.go b/internal/unifiedresources/top_level_systems_basis_branchcov0720am_test.go new file mode 100644 index 000000000..7cfaf942e --- /dev/null +++ b/internal/unifiedresources/top_level_systems_basis_branchcov0720am_test.go @@ -0,0 +1,106 @@ +package unifiedresources + +import ( + "strings" + "testing" +) + +// TestBranchcov0720am_TopLevelSystemIdentityMatchBasis exercises every +// conditional arm of topLevelSystemIdentityMatchBasis: the two recognized +// identity-match signals ("dmi_uuid", "hostname+mac") and the fallback for +// any other (including empty) signal. +// +// The function is a pure signal classifier that returns a human-readable +// basis string. Rather than pinning exact literals (which would be a brittle +// change-detector), these subtests assert the *classifier* behavior: +// +// - every recognized signal produces a distinct, non-empty classification +// (i.e. the function discriminates between the known signals); +// - any unrecognized signal collapses to the single shared fallback; +// - the fallback is distinct from every recognized-signal output (so the +// default arm is genuinely a third branch, not a silent alias); +// - the function is deterministic for identical input. +func TestBranchcov0720am_TopLevelSystemIdentityMatchBasis(t *testing.T) { + recognized := []string{"dmi_uuid", "hostname+mac"} + unknown := []string{ + "", + " ", + "machine-id", + "agent-id", + "canonical-primary-id", + "resource-id", + "ip", + "something-the-classifier-does-not-know", + } + + t.Run("recognized signals each yield a distinct non-empty classification", func(t *testing.T) { + seen := make(map[string]string, len(recognized)) + for _, signal := range recognized { + got := topLevelSystemIdentityMatchBasis(signal) + if strings.TrimSpace(got) == "" { + t.Fatalf("signal %q returned empty basis", signal) + } + if existing, ok := seen[got]; ok { + t.Fatalf( + "signals %q and %q collapsed to the same basis %q; "+ + "recognized signals must remain distinguishable", + existing, signal, got, + ) + } + seen[got] = signal + } + if len(seen) != len(recognized) { + t.Fatalf("expected %d distinct classifications, got %d", len(recognized), len(seen)) + } + }) + + t.Run("unknown signals all collapse to the single fallback classification", func(t *testing.T) { + fallback := topLevelSystemIdentityMatchBasis("__definitely-not-a-known-signal__") + if strings.TrimSpace(fallback) == "" { + t.Fatalf("fallback basis is empty; classifier must always classify") + } + for _, signal := range unknown { + got := topLevelSystemIdentityMatchBasis(signal) + if got != fallback { + t.Fatalf( + "unknown signal %q returned %q, expected the shared fallback %q", + signal, got, fallback, + ) + } + } + }) + + t.Run("fallback arm is a real third branch distinct from recognized arms", func(t *testing.T) { + fallback := topLevelSystemIdentityMatchBasis("nope") + for _, signal := range recognized { + if got := topLevelSystemIdentityMatchBasis(signal); got == fallback { + t.Fatalf( + "recognized signal %q produced the same basis as the fallback %q; "+ + "default arm is not exercising a distinct branch", + signal, fallback, + ) + } + } + }) + + t.Run("classification is deterministic for identical input", func(t *testing.T) { + for _, signal := range append(append([]string{}, recognized...), unknown...) { + first := topLevelSystemIdentityMatchBasis(signal) + second := topLevelSystemIdentityMatchBasis(signal) + if first != second { + t.Fatalf( + "signal %q classified non-deterministically: %q then %q", + signal, first, second, + ) + } + } + }) + + t.Run("result never carries leading/trailing whitespace", func(t *testing.T) { + for _, signal := range append(append([]string{}, recognized...), "__unknown__") { + if got := topLevelSystemIdentityMatchBasis(signal); got != strings.TrimSpace(got) { + t.Fatalf("signal %q returned padded basis %q", signal, got) + } + } + }) +} diff --git a/internal/vmware/activity_changes_branchcov0720am_test.go b/internal/vmware/activity_changes_branchcov0720am_test.go new file mode 100644 index 000000000..f3b8c6d95 --- /dev/null +++ b/internal/vmware/activity_changes_branchcov0720am_test.go @@ -0,0 +1,446 @@ +package vmware + +import ( + "sort" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +// TestBranchcov0720am_FixtureActivityChanges drives every conditional arm of +// FixtureActivityChanges -> activityChangesFromSnapshot -> entityActivityChanges: +// +// - the empty-snapshot early-return path (no hosts/vms/datastores/networks to +// iterate, leaving the result slice at length zero), +// - each of the four per-entity iteration arms (hosts, vms, datastores, +// networks) projecting tasks and events onto the timeline, +// - the `if change != nil` filter arm in entityActivityChanges, reached when a +// task/event with no native id, title, or message causes +// BuildPlatformActivityChange to return nil, +// - both arms of the sort comparator (ObservedAt differs vs. ObservedAt equal +// so the ID tie-breaker fires), +// - and independence of the returned slice from a fresh call (mutating one +// result must not bleed into the next). +// +// The projection is behavioural: we assert classification (Kind/SourceType/ +// SourceAdapter), key metadata plumbing (connectionId/entityType/ +// managedObjectId), ordering outcomes, and filter outcomes rather than pinning +// exact literal change IDs. +func TestBranchcov0720am_FixtureActivityChanges(t *testing.T) { + // Use fixed, non-zero instants so ObservedAt is deterministic across the + // sort-driven subtests. Use a non-UTC zone to also exercise the UTC + // normalisation performed by BuildPlatformActivityChange. + pst := time.FixedZone("PST", -8*3600) + earlier := time.Date(2026, time.July, 20, 9, 0, 0, 0, pst) // 17:00 UTC + later := time.Date(2026, time.July, 20, 11, 0, 0, 0, pst) // 19:00 UTC + shared := time.Date(2026, time.July, 20, 12, 0, 0, 0, pst) // 20:00 UTC + + t.Run("empty_snapshot_returns_empty_non_nil_slice", func(t *testing.T) { + got := FixtureActivityChanges(InventorySnapshot{ConnectionID: "vc-empty"}) + if got == nil { + t.Fatalf("FixtureActivityChanges(empty) returned nil; want a non-nil empty slice so callers can range safely") + } + if len(got) != 0 { + t.Fatalf("FixtureActivityChanges(empty) returned %d changes; want 0", len(got)) + } + }) + + t.Run("zero_valued_snapshot_returns_empty_non_nil_slice", func(t *testing.T) { + // A brand-new InventorySnapshot has no ConnectionID and no entities; the + // four iteration loops should all be no-ops and the sorter should leave + // the empty slice alone. + got := FixtureActivityChanges(InventorySnapshot{}) + if got == nil { + t.Fatalf("FixtureActivityChanges(zero) returned nil; want non-nil empty slice") + } + if len(got) != 0 { + t.Fatalf("FixtureActivityChanges(zero) returned %d changes; want 0", len(got)) + } + }) + + t.Run("host_with_populated_task_emits_activity_change", func(t *testing.T) { + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + Hosts: []InventoryHost{{ + Host: "host-1", + RecentTasks: []InventoryTask{{Task: "task-1", Name: "PowerOn", State: "success", StartedAt: earlier}}, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 1 { + t.Fatalf("expected 1 change for a host with one populated task, got %d", len(got)) + } + c := got[0] + if c.Kind != unifiedresources.ChangeActivity { + t.Errorf("change.Kind = %q, want %q", c.Kind, unifiedresources.ChangeActivity) + } + if c.SourceType != unifiedresources.SourcePlatformEvent { + t.Errorf("change.SourceType = %q, want %q", c.SourceType, unifiedresources.SourcePlatformEvent) + } + if c.SourceAdapter != unifiedresources.AdapterVMware { + t.Errorf("change.SourceAdapter = %q, want %q", c.SourceAdapter, unifiedresources.AdapterVMware) + } + // The resource id must thread snapshot.ConnectionID + "host" + host.Host + // through; assert the components appear rather than pinning the exact + // canonical string so this remains a behavioural assertion. + if !strings.Contains(c.ResourceID, "vc-1") || !strings.Contains(c.ResourceID, "host-1") || !strings.Contains(c.ResourceID, "host") { + t.Errorf("change.ResourceID = %q; want it to carry connection/entity/managed-object components", c.ResourceID) + } + // Metadata must carry the provenance keys produced by entityActivityChanges. + if got := metadataString(c.Metadata, "vmwareConnectionId"); got != "vc-1" { + t.Errorf("metadata vmwareConnectionId = %q, want %q", got, "vc-1") + } + if got := metadataString(c.Metadata, "vmwareEntityType"); got != "host" { + t.Errorf("metadata vmwareEntityType = %q, want %q", got, "host") + } + if got := metadataString(c.Metadata, "vmwareManagedObjectId"); got != "host-1" { + t.Errorf("metadata vmwareManagedObjectId = %q, want %q", got, "host-1") + } + if got := metadataString(c.Metadata, "vmwareTask"); got != "task-1" { + t.Errorf("metadata vmwareTask = %q, want %q", got, "task-1") + } + // CompletedAt falls back to StartedAt via firstNonZeroTime, and then is + // normalised to UTC by BuildPlatformActivityChange. + wantOccurredUTC := earlier.UTC() + if c.OccurredAt == nil || !c.OccurredAt.Equal(wantOccurredUTC) { + t.Errorf("change.OccurredAt = %v, want %v (CompletedAt->StartedAt fallback in UTC)", c.OccurredAt, wantOccurredUTC) + } + }) + + t.Run("host_with_populated_event_emits_activity_change", func(t *testing.T) { + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + Hosts: []InventoryHost{{ + Host: "host-1", + RecentEvents: []InventoryEvent{{ + Event: "event-1", + Type: "HostConnectedEvent", + Message: "host reconnected", + User: "svc-pulse", + CreatedAt: earlier, + }}, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 1 { + t.Fatalf("expected 1 change for a host with one populated event, got %d", len(got)) + } + c := got[0] + if c.Kind != unifiedresources.ChangeActivity { + t.Errorf("change.Kind = %q, want %q", c.Kind, unifiedresources.ChangeActivity) + } + if got := metadataString(c.Metadata, "vmwareEvent"); got != "event-1" { + t.Errorf("metadata vmwareEvent = %q, want %q", got, "event-1") + } + if got := metadataString(c.Metadata, "vmwareEventType"); got != "HostConnectedEvent" { + t.Errorf("metadata vmwareEventType = %q, want %q", got, "HostConnectedEvent") + } + if got := metadataString(c.Metadata, "vmwareEventUser"); got != "svc-pulse" { + t.Errorf("metadata vmwareEventUser = %q, want %q", got, "svc-pulse") + } + if c.Actor != "svc-pulse" { + t.Errorf("change.Actor = %q, want %q", c.Actor, "svc-pulse") + } + }) + + t.Run("host_with_whitespace_only_task_is_filtered_out", func(t *testing.T) { + // A task whose Task/Name/ErrorMessage all trim to empty causes + // BuildPlatformActivityChange to return nil, so entityActivityChanges + // drops it via `if change != nil`. This is the only way to reach that + // filter arm for tasks. + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + Hosts: []InventoryHost{{ + Host: "host-1", + RecentTasks: []InventoryTask{ + {Task: " ", Name: "\t", ErrorMessage: " "}, + }, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 0 { + t.Fatalf("expected whitespace-only task to be filtered out, got %d changes: %+v", len(got), got) + } + }) + + t.Run("host_with_whitespace_only_event_is_filtered_out", func(t *testing.T) { + // An event whose Event/Type/Message all trim to empty causes + // BuildPlatformActivityChange to return nil, exercising the filter arm + // for events. + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + Hosts: []InventoryHost{{ + Host: "host-1", + RecentEvents: []InventoryEvent{ + {Event: " ", Type: "\t", Message: ""}, + }, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 0 { + t.Fatalf("expected whitespace-only event to be filtered out, got %d changes: %+v", len(got), got) + } + }) + + t.Run("vm_datastore_network_each_project_one_change_per_task", func(t *testing.T) { + // Exercises all four per-entity iteration arms of + // activityChangesFromSnapshot in one snapshot. Each entity carries a + // single populated task; the managed object id differs per entity so we + // can prove the right entity type threaded its id into ResourceID. + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + VMs: []InventoryVM{{ + VM: "vm-1", + RecentTasks: []InventoryTask{{Task: "vm-task", Name: "snapshot", StartedAt: earlier}}, + }}, + Datastores: []InventoryDatastore{{ + Datastore: "ds-1", + RecentTasks: []InventoryTask{{Task: "ds-task", Name: "rescan", StartedAt: earlier}}, + }}, + Networks: []InventoryNetwork{{ + Network: "net-1", + RecentTasks: []InventoryTask{{Task: "net-task", Name: "refresh", StartedAt: earlier}}, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 3 { + t.Fatalf("expected 3 changes (one per entity type), got %d", len(got)) + } + seen := map[string]bool{} + for _, c := range got { + seen[c.ResourceID] = true + } + for _, want := range []string{"vm-1", "ds-1", "net-1"} { + found := false + for rid := range seen { + if strings.Contains(rid, want) { + found = true + break + } + } + if !found { + t.Errorf("expected a change carrying managed object id %q; ResourceIDs seen = %v", want, seen) + } + } + }) + + t.Run("changes_sorted_by_observed_at_descending", func(t *testing.T) { + // Two tasks at distinct UTC instants; the comparator must take the + // `!Equal` arm and order the later-OccurredAt change first. + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + Hosts: []InventoryHost{{ + Host: "host-1", + RecentTasks: []InventoryTask{ + {Task: "earlier-task", Name: "earlier", StartedAt: earlier}, + {Task: "later-task", Name: "later", StartedAt: later}, + }, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 2 { + t.Fatalf("expected 2 changes, got %d", len(got)) + } + if !got[0].ObservedAt.After(got[1].ObservedAt) { + t.Errorf("expected changes sorted by ObservedAt descending; got[0]=%v got[1]=%v", got[0].ObservedAt, got[1].ObservedAt) + } + if got := metadataString(got[0].Metadata, "vmwareTask"); got != "later-task" { + t.Errorf("expected first change to be the later task; got vmwareTask=%q", got) + } + }) + + t.Run("ties_broken_by_id_descending", func(t *testing.T) { + // Two tasks at the same instant force the comparator's `Equal` arm, + // which falls through to `changes[i].ID > changes[j].ID`. We craft two + // tasks with distinct native ids so they produce distinct change IDs; + // whichever ID sorts lexicographically higher must come first. + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + Hosts: []InventoryHost{{ + Host: "host-1", + RecentTasks: []InventoryTask{ + {Task: "task-zzz", Name: "same-time", StartedAt: shared}, + {Task: "task-aaa", Name: "same-time", StartedAt: shared}, + }, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 2 { + t.Fatalf("expected 2 changes, got %d", len(got)) + } + if !got[0].ObservedAt.Equal(got[1].ObservedAt) { + t.Fatalf("precondition failed: expected equal ObservedAt to force the ID tie-breaker; got %v vs %v", got[0].ObservedAt, got[1].ObservedAt) + } + // The ID that is lexicographically greater must sort first when the + // ObservedAt values tie. + if got[0].ID < got[1].ID { + t.Errorf("expected ID tie-breaker in descending order; got[0].ID=%q < got[1].ID=%q", got[0].ID, got[1].ID) + } + // Cross-check by re-sorting the same changes by ID desc and comparing; + // this keeps the assertion robust to comparator quirks while still + // pinning behaviour. + byID := append([]unifiedresources.ResourceChange(nil), got...) + sort.SliceStable(byID, func(i, j int) bool { return byID[i].ID > byID[j].ID }) + if byID[0].ID != got[0].ID || byID[1].ID != got[1].ID { + t.Errorf("tie-break order does not match ID desc; got=%v,%v want=%v,%v", got[0].ID, got[1].ID, byID[0].ID, byID[1].ID) + } + }) + + t.Run("returned_slice_is_independent_of_subsequent_calls", func(t *testing.T) { + // Mutating the slice returned by one call (including a metadata map + // value) must not leak into a second call on the same snapshot. This + // proves FixtureActivityChanges builds fresh state per invocation + // rather than aliasing a cached slice or the input snapshot's maps. + snapshot := InventorySnapshot{ + ConnectionID: "vc-1", + Hosts: []InventoryHost{{ + Host: "host-1", + RecentTasks: []InventoryTask{{Task: "task-1", Name: "PowerOn", StartedAt: earlier}}, + }}, + } + first := FixtureActivityChanges(snapshot) + if len(first) != 1 { + t.Fatalf("precondition: expected 1 change, got %d", len(first)) + } + originalID := first[0].ID + originalTask := metadataString(first[0].Metadata, "vmwareTask") + + // Mutate the first result in-place. + first[0].ID = "mutated-id" + if m, ok := first[0].Metadata["vmwareTask"].(string); ok { + first[0].Metadata["vmwareTask"] = m + "-mutated" + } + first[0].Metadata["new-key"] = "injected" + + second := FixtureActivityChanges(snapshot) + if len(second) != 1 { + t.Fatalf("expected second call to also return 1 change, got %d", len(second)) + } + if second[0].ID != originalID { + t.Errorf("second call ID = %q, want %q (first call mutation leaked)", second[0].ID, originalID) + } + if got := metadataString(second[0].Metadata, "vmwareTask"); got != originalTask { + t.Errorf("second call vmwareTask metadata = %q, want %q (metadata mutation leaked)", got, originalTask) + } + if _, leaked := second[0].Metadata["new-key"]; leaked { + t.Errorf("second call metadata contains injected key 'new-key' (metadata map aliased across calls)") + } + }) + + t.Run("whitespace_in_task_fields_is_trimmed_in_emitted_change", func(t *testing.T) { + // entityActivityChanges applies strings.TrimSpace to every field before + // building the change. Driving padded inputs proves the trimming arm is + // actually exercised end-to-end (rather than asserting on a TrimSpace + // constant). + snapshot := InventorySnapshot{ + ConnectionID: " vc-1 ", + Hosts: []InventoryHost{{ + Host: "\thost-1\n", + RecentTasks: []InventoryTask{{ + Task: " task-1 ", + Name: " PowerOn ", + State: " success ", + ErrorMessage: " no-error ", + StartedAt: earlier, + }}, + }}, + } + got := FixtureActivityChanges(snapshot) + if len(got) != 1 { + t.Fatalf("expected 1 change, got %d", len(got)) + } + c := got[0] + if got := metadataString(c.Metadata, "vmwareConnectionId"); got != "vc-1" { + t.Errorf("vmwareConnectionId not trimmed: %q", got) + } + if got := metadataString(c.Metadata, "vmwareManagedObjectId"); got != "host-1" { + t.Errorf("vmwareManagedObjectId not trimmed: %q", got) + } + if got := metadataString(c.Metadata, "vmwareTask"); got != "task-1" { + t.Errorf("vmwareTask not trimmed: %q", got) + } + if got := metadataString(c.Metadata, "vmwareTaskName"); got != "PowerOn" { + t.Errorf("vmwareTaskName not trimmed: %q", got) + } + if got := metadataString(c.Metadata, "vmwareTaskState"); got != "success" { + t.Errorf("vmwareTaskState not trimmed: %q", got) + } + if got := metadataString(c.Metadata, "vmwareTaskError"); got != "no-error" { + t.Errorf("vmwareTaskError not trimmed: %q", got) + } + }) +} + +// TestBranchcov0720am_ConnectionError_Error exercises both arms of +// (*ConnectionError).Error(): the nil-receiver guard (`if e == nil`) and the +// straight-line `return e.Message` path. The behavioural contract is "Error() +// returns e.Message verbatim when non-nil, and the empty string when the +// receiver is nil" -- so we assert against that contract rather than against a +// formatted string that might smuggle in Category. +func TestBranchcov0720am_ConnectionError_Error(t *testing.T) { + t.Run("nil_receiver_returns_empty_string", func(t *testing.T) { + // Calling Error() on a nil *ConnectionError exercises the `if e == nil` + // arm. This is a real call site: package-internal code stores + // *ConnectionError in `err` variables that can be nil-typed at runtime. + var e *ConnectionError + got := e.Error() + if got != "" { + t.Errorf("nil (*ConnectionError).Error() = %q, want %q", got, "") + } + }) + + t.Run("populated_receiver_returns_message_ignoring_category", func(t *testing.T) { + // A ConnectionError carries both Category and Message; Error() must + // surface Message only. Asserting that Category never appears in the + // result distinguishes the real implementation from a hypothetical + // "Category: Message" formatter. + const category = "endpoint" + const message = "VMware VI JSON API service-instance response was not valid JSON" + e := &ConnectionError{Category: category, Message: message} + got := e.Error() + if got != message { + t.Errorf("Error() = %q, want exactly Message %q", got, message) + } + if strings.Contains(got, category) { + t.Errorf("Error() = %q unexpectedly embeds Category %q; want Message verbatim", got, category) + } + }) + + t.Run("empty_message_returns_empty_string", func(t *testing.T) { + // A non-nil ConnectionError with an empty Message still takes the + // `return e.Message` arm; the result must be the empty string rather + // than a fallback like Category or a static placeholder. + e := &ConnectionError{Category: "auth"} + if got := e.Error(); got != "" { + t.Errorf("Error() = %q, want empty string when Message is empty", got) + } + }) + + t.Run("typed_nil_in_error_interface_still_returns_empty", func(t *testing.T) { + // A typed-nil pointer stored in an `error` interface is a common shape + // for `var err error = (*ConnectionError)(nil)`; calling Error() via + // the interface must still hit the nil-receiver arm rather than panic. + var ifaceErr error = (*ConnectionError)(nil) + got := ifaceErr.Error() + if got != "" { + t.Errorf("typed-nil error.Error() = %q, want empty string", got) + } + }) +} + +// metadataString reads a string-valued key from a change metadata map without +// panicking on missing keys or non-string values; it returns "" if the key is +// absent so callers can compare against an expected value. +func metadataString(meta map[string]any, key string) string { + if meta == nil { + return "" + } + v, ok := meta[key] + if !ok { + return "" + } + s, _ := v.(string) + return s +}