From e39a8fedcf9f614c43daeb8e81d48385e217c8f6 Mon Sep 17 00:00:00 2001 From: Costa Tsaousis Date: Thu, 4 Jun 2026 06:41:47 +0300 Subject: [PATCH] test config helper contracts --- ...quality-complexity-duplication-coverage.md | 129 ++++++++++++- pkg/config/helper_contract_test.go | 169 ++++++++++++++++++ 2 files changed, 295 insertions(+), 3 deletions(-) create mode 100644 pkg/config/helper_contract_test.go diff --git a/.agents/sow/current/SOW-0102-20260603-quality-complexity-duplication-coverage.md b/.agents/sow/current/SOW-0102-20260603-quality-complexity-duplication-coverage.md index 9e1abaf..dd6a5fa 100644 --- a/.agents/sow/current/SOW-0102-20260603-quality-complexity-duplication-coverage.md +++ b/.agents/sow/current/SOW-0102-20260603-quality-complexity-duplication-coverage.md @@ -4,7 +4,7 @@ Status: in-progress -Sub-state: twenty-seventh implementation slice validated; downloader canonical coverage PR pending +Sub-state: twenty-eighth implementation slice validated; config helper coverage PR pending ## Requirements @@ -2151,6 +2151,53 @@ Open decisions: - No new user design decision is required because the slice is behavior-preserving test coverage under the previously approved quality plan. +## Slice 28 Results + +Changes made: + +- Added behavior tests for config helper contracts in `pkg/config/helper_contract_test.go`. +- Covered category lookup, default-public category behavior, explicit private category behavior, and category ordering. +- Covered nil-config category helper behavior. +- Covered sorted artifact names and artifact child lookup respecting `SourceOrder`. +- Covered retention-window URL parser success and error cases. +- Covered `ParseMergeURL` additive-only compatibility behavior. +- Production code was unchanged. + +Measured result: + +- Baseline: `pkg/config` coverage was `71.2%` by `go test` output. +- After tests: `pkg/config` coverage is `75.4%` by `go test` output. +- `CategoryByName`, `CategoriesOrdered`, `PublicCategoriesOrdered`, `CategoryIsPublic`, `CategoryDefinition.IsPublic`, and `categoriesOrdered` moved from `0.0%` to `100.0%`. +- `ArtifactChildren` moved from `0.0%` to `88.9%`. +- `SortedArtifactNames` moved from `0.0%` to `100.0%`. +- `ParseRetentionWindowURL` moved from `0.0%` to `94.4%`. +- `ParseMergeURL` moved from `0.0%` to `100.0%`. +- Root coverage by `go tool cover -func=coverage.out` moved from `73.2%` to `73.4%`. +- `tools/archposture` after this slice: source files `635`, source lines `127725`, large files `49`, large functions `25`, and production large functions `0`. + +Tests or equivalent validation: + +- `go test ./pkg/config`: passed. +- `go test -coverprofile=/tmp/update-ipsets-config-slice28.cover -covermode=atomic ./pkg/config`: passed, `75.4%`. +- `go tool cover -func=/tmp/update-ipsets-config-slice28.cover`: passed; targeted helper coverage listed above. +- `go run ./tools/archposture -root . > /tmp/update-ipsets-archposture-slice28.json`: passed. +- `make lint`: passed. +- `make staticcheck`: passed. +- `make golangci-lint`: passed with `0 issues`. +- `CI=true make coverage`: passed, root total `73.4%`. +- `make test-strict`: passed. +- `git diff --check`: passed. +- Durable-artifact forbidden-name scan over added diff lines found no newly added personal name, authorship, tool, or vendor-attribution text. + +Artifact maintenance gate: + +- AGENTS.md: no update needed. +- Runtime project skills: no update needed; no new durable process rule was found. +- Specs: no update needed; config helper behavior is unchanged. +- End-user/operator docs: no update needed. +- End-user/operator skills: no update needed. +- SOW lifecycle: remains in `.agents/sow/current/`; Slice 28 is validated and pending PR merge. + ## Slice 27 Results Changes made: @@ -2199,7 +2246,83 @@ Artifact maintenance gate: - Specs: no update needed; canonical downloader behavior is unchanged. - End-user/operator docs: no update needed. - End-user/operator skills: no update needed. -- SOW lifecycle: remains in `.agents/sow/current/`; Slice 27 is validated and pending PR merge. +- SOW lifecycle: remains in `.agents/sow/current/`; Slice 27 merged through PR #31 as merge commit `dec6b9395e80094cb1c663b0fbb531946249c133`. + +## Pre-Implementation Gate - Slice 28 + +Status: ready. + +Problem / root-cause model: + +- Facts: after the Slice 27 merge, local `tools/archposture` still reports zero production large functions. +- Facts: `go test -coverprofile=/tmp/update-ipsets-config-slice28-baseline.cover -covermode=atomic ./pkg/config` reports several clear config helper gaps: `CategoryByName`, `CategoriesOrdered`, `PublicCategoriesOrdered`, `CategoryIsPublic`, `CategoryDefinition.IsPublic`, and `categoriesOrdered` at `0.0%`; `ArtifactChildren` and `SortedArtifactNames` at `0.0%`; `ParseRetentionWindowURL` and `ParseMergeURL` at `0.0%`. +- Working theory: config helper behavior is a low-risk coverage slice because these helpers expose deterministic ordering, public/private category semantics, artifact child lookup, and URL parsing contracts without requiring engine, network, or filesystem side effects beyond temporary test inputs. + +Evidence reviewed: + +- `pkg/config/category_registry.go` +- `pkg/config/artifacts.go` +- `pkg/config/expand.go` +- `pkg/config/config_test.go` +- `pkg/config/catalog_verify_test.go` +- `/tmp/update-ipsets-config-slice28-baseline.cover` +- `/tmp/update-ipsets-archposture-after-pr30.json` +- Project coding, testing, hygiene, Go best-practices, Go behavioral-testing, and content-surface skills. + +Affected contracts and surfaces: + +- Category lookup, default-public category behavior, explicit private category behavior, and category ordering. +- Artifact child lookup order and sorted artifact names. +- Retention-window URL parsing and merge URL compatibility wrapper behavior. +- SOW and tests only; no production code, docs, or specs are expected to change. + +Existing patterns to reuse: + +- Existing `pkg/config` same-package tests. +- Existing table-driven test style and synthetic `Config` values. +- Existing `SourceOrder` behavior for deterministic source ordering. +- Existing URL scheme constants and parsing helpers. + +Risk and blast radius: + +- This slice should be test-only. +- Tests must assert documented helper behavior, not implementation details such as map iteration. +- Category ordering tests must include tie-breaks so future regressions are observable. +- No runtime config semantics, catalog files, downloader behavior, scheduler behavior, public serving, install behavior, or UI behavior should change. + +Sensitive data handling plan: + +- This slice uses only synthetic category, artifact, and source names. +- No secrets, tokens, cookies, private endpoints, customer data, or personal data are needed. +- Durable artifacts will record only file paths, metrics, validation outcomes, and sanitized command evidence. + +Implementation plan: + +1. Add category registry behavior tests for lookup, default-public behavior, explicit private behavior, and ordering tie-breaks. +2. Add artifact helper tests for sorted artifact names and `ArtifactChildren` respecting `SourceOrder`. +3. Add URL parser tests for `ParseRetentionWindowURL` success/failure cases and `ParseMergeURL` additive-only compatibility behavior. +4. Keep production code unchanged. + +Validation plan: + +- Run `go test ./pkg/config`. +- Run `go test -coverprofile=/tmp/update-ipsets-config-slice28.cover -covermode=atomic ./pkg/config` and inspect `go tool cover -func`. +- Run `go run ./tools/archposture -root . > /tmp/update-ipsets-archposture-slice28.json`. +- Run `make lint`, `make staticcheck`, `make golangci-lint`, `CI=true make coverage`, and `make test-strict`. +- Run whitespace and durable-artifact forbidden-name scans over the changed files before commit. + +Artifact impact plan: + +- AGENTS.md: no update expected. +- Runtime project skills: no update needed unless a repeatable config-helper testing lesson is found. +- Specs: no update expected because config helper behavior is unchanged. +- End-user/operator docs: no update expected. +- End-user/operator skills: no update expected. +- SOW lifecycle: this SOW remains in `.agents/sow/current/`; Slice 28 results will be recorded after validation. + +Open decisions: + +- No new user design decision is required because the slice is behavior-preserving test coverage under the previously approved quality plan. ## Slice 25 Results @@ -3524,7 +3647,7 @@ Open decisions: ## Outcome -First through twenty-sixth implementation slices are complete, validated locally, and merged. The twenty-seventh implementation slice is complete and validated locally. The SOW remains open for the next focused coverage, complexity, or duplication slice. +First through twenty-seventh implementation slices are complete, validated locally, and merged. The twenty-eighth implementation slice is complete and validated locally. The SOW remains open for the next focused coverage, complexity, or duplication slice. ## Lessons Extracted diff --git a/pkg/config/helper_contract_test.go b/pkg/config/helper_contract_test.go new file mode 100644 index 0000000..853f5a5 --- /dev/null +++ b/pkg/config/helper_contract_test.go @@ -0,0 +1,169 @@ +package config + +import ( + "reflect" + "testing" +) + +func TestCategoryRegistryHelpers(t *testing.T) { + private := false + public := true + cfg := New() + cfg.Categories = map[string]CategoryDefinition{ + "hidden": { + Label: "Hidden", + Description: "private category", + SortOrder: 1, + Public: &private, + }, + "alpha_a": { + Label: "Alpha", + Description: "first alpha category", + SortOrder: 1, + Public: &public, + }, + "alpha_b": { + Label: "Alpha", + Description: "second alpha category", + SortOrder: 1, + }, + "beta": { + Label: "Beta", + Description: "beta category", + SortOrder: 2, + }, + } + + def, ok := cfg.CategoryByName("hidden") + if !ok || def.Label != "Hidden" { + t.Fatalf("CategoryByName(hidden) = %+v, %v", def, ok) + } + if _, ok := cfg.CategoryByName("missing"); ok { + t.Fatal("CategoryByName(missing) ok = true, want false") + } + if (&Config{}).CategoryIsPublic("") { + t.Fatal("blank category should not be public") + } + if !cfg.CategoryIsPublic("unknown") { + t.Fatal("unknown categories should default to public") + } + if cfg.CategoryIsPublic("hidden") { + t.Fatal("explicit private category should not be public") + } + if !cfg.CategoryIsPublic("alpha_b") { + t.Fatal("category with nil Public should be public") + } + + if got, want := categoryNames(cfg.CategoriesOrdered()), []string{"alpha_a", "alpha_b", "hidden", "beta"}; !reflect.DeepEqual(got, want) { + t.Fatalf("CategoriesOrdered names = %v, want %v", got, want) + } + if got, want := categoryNames(cfg.PublicCategoriesOrdered()), []string{"alpha_a", "alpha_b", "beta"}; !reflect.DeepEqual(got, want) { + t.Fatalf("PublicCategoriesOrdered names = %v, want %v", got, want) + } +} + +func TestCategoryRegistryNilConfig(t *testing.T) { + var cfg *Config + if _, ok := cfg.CategoryByName("anything"); ok { + t.Fatal("nil CategoryByName ok = true, want false") + } + if cfg.CategoriesOrdered() != nil { + t.Fatal("nil CategoriesOrdered should return nil") + } + if cfg.PublicCategoriesOrdered() != nil { + t.Fatal("nil PublicCategoriesOrdered should return nil") + } + if !cfg.CategoryIsPublic("anything") { + t.Fatal("nil config should treat unknown named category as public") + } + if (CategoryDefinition{}).IsPublic() != true { + t.Fatal("zero CategoryDefinition should default to public") + } +} + +func TestArtifactHelpers(t *testing.T) { + cfg := New() + cfg.Artifacts = map[string]*Artifact{ + "zeta": {Name: "zeta"}, + "alpha": {Name: "alpha"}, + "mid": {Name: "mid"}, + } + cfg.Sources = map[string]*Source{ + "first": {Name: "first", ArtifactParent: "alpha"}, + "second": {Name: "second", ArtifactParent: "alpha"}, + "other": {Name: "other", ArtifactParent: "zeta"}, + "plain": {Name: "plain"}, + } + cfg.SourceOrder = []string{"second", "plain", "first", "other"} + + if got, want := SortedArtifactNames(cfg), []string{"alpha", "mid", "zeta"}; !reflect.DeepEqual(got, want) { + t.Fatalf("SortedArtifactNames = %v, want %v", got, want) + } + if SortedArtifactNames(nil) != nil { + t.Fatal("SortedArtifactNames(nil) should return nil") + } + children := cfg.ArtifactChildren("alpha") + if got, want := sourceNames(children), []string{"second", "first"}; !reflect.DeepEqual(got, want) { + t.Fatalf("ArtifactChildren(alpha) = %v, want %v", got, want) + } + if got := cfg.ArtifactChildren("missing"); len(got) != 0 { + t.Fatalf("ArtifactChildren(missing) len = %d, want 0", len(got)) + } +} + +func TestParseRetentionWindowURL(t *testing.T) { + parent, minutes, err := ParseRetentionWindowURL(InternalRetentionWindowScheme + "?parent=base&minutes=1440") + if err != nil { + t.Fatalf("ParseRetentionWindowURL(valid): %v", err) + } + if parent != "base" || minutes != 1440 { + t.Fatalf("ParseRetentionWindowURL(valid) = %q, %d", parent, minutes) + } + + tests := []string{ + "internal://other?parent=base&minutes=1440", + InternalRetentionWindowScheme + "?minutes=1440", + InternalRetentionWindowScheme + "?parent=base", + InternalRetentionWindowScheme + "?parent=base&minutes=not-an-int", + InternalRetentionWindowScheme + "?parent=base&minutes=0", + } + for _, rawURL := range tests { + t.Run(rawURL, func(t *testing.T) { + if _, _, err := ParseRetentionWindowURL(rawURL); err == nil { + t.Fatal("ParseRetentionWindowURL error = nil, want error") + } + }) + } +} + +func TestParseMergeURLCompatibility(t *testing.T) { + inputs, err := ParseMergeURL(InternalMergeScheme + "?inputs=alpha,,beta&exclude=private") + if err != nil { + t.Fatalf("ParseMergeURL(valid): %v", err) + } + if got, want := inputs, []string{"alpha", "beta"}; !reflect.DeepEqual(got, want) { + t.Fatalf("ParseMergeURL inputs = %v, want %v", got, want) + } + if _, err := ParseMergeURL("internal://other?inputs=alpha"); err == nil { + t.Fatal("ParseMergeURL(wrong scheme) error = nil, want error") + } + if _, err := ParseMergeURL(InternalMergeScheme + "?exclude=private"); err == nil { + t.Fatal("ParseMergeURL(missing inputs) error = nil, want error") + } +} + +func categoryNames(categories []NamedCategory) []string { + names := make([]string, 0, len(categories)) + for _, category := range categories { + names = append(names, category.Name) + } + return names +} + +func sourceNames(sources []*Source) []string { + names := make([]string, 0, len(sources)) + for _, source := range sources { + names = append(names, source.Name) + } + return names +}