From c7fb5a27bba3d0d12c0cab148a6dd231312cae34 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 12 Oct 2026 00:16:47 +0000 Subject: [PATCH] Test comparison preparation through provider behavior --- docs/roadmap/remediation.md | 15 +++- .../ckconfig/comparison_preparation_test.go | 78 +++++++++++-------- 2 files changed, 58 insertions(+), 35 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index e39812b..2594073 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Configuration comparison review remediation -Status: two findings complete; two open. This document tracks required +Status: three findings complete; one open. This document tracks required follow-up work and its completion evidence. ## Review baseline and scope @@ -122,7 +122,7 @@ No runtime enforcement or consumer requirement was added. ## R-3: Make preparation tests reject failure without freezing private storage -**Status:** Open. **Priority:** Medium. **Category:** Test quality / +**Status:** Complete. **Priority:** Medium. **Category:** Test quality / inadvertently strengthened implementation invariants. **Evidence:** In @@ -151,6 +151,17 @@ rejected by the test. Tests protect the paired provider/lifecycle contract without requiring a particular private plan layout. Reuse existing behavioral coverage where it already proves the relevant fact. +**Completed 2026-10-12:** The valid equal-selection test now requires +`OutcomeSuccess`, so an operational failure cannot pass. The storage-layout test +was replaced with an assembled invocation that observes the provider request, +correlates each input with its checked selection, verifies both checks precede +shared state and provider execution, and verifies ordinary acquisition is unused. +It does not inspect the private plan or assign particular factory IDs to sides. +Existing presence, conversion, collection ownership, zero-value, alias, and +failure-classification cases remain. `go test ./...` passed in +`integrations/ckconfig`, including the simple consumer; formatting, diff, and +whitespace review passed. + ## R-4: Reconcile the feature roadmap's completion status **Status:** Open. **Priority:** Low. **Category:** Documentation divergence. diff --git a/integrations/ckconfig/comparison_preparation_test.go b/integrations/ckconfig/comparison_preparation_test.go index b03fed9..154e497 100644 --- a/integrations/ckconfig/comparison_preparation_test.go +++ b/integrations/ckconfig/comparison_preparation_test.go @@ -93,8 +93,8 @@ func TestComparisonPreparesEqualSelectionsWithSeparateTypedInputs(t *testing.T) "config", "diff", "--config", "same.yml", "--against-config", "same.yml", "--overlay=", "--enabled=false", "--limit=0", "--tag", "alpha", "--tag", "beta", }, commandkit.Streams{Out: io.Discard, Err: io.Discard}) - if outcome.Kind == commandkit.OutcomeUsage { - t.Fatalf("valid comparison inputs rejected: %#v", outcome) + if outcome.Kind != commandkit.OutcomeSuccess { + t.Fatalf("valid comparison outcome = %#v", outcome) } if inputCalls != 2 || checkCalls != 2 || stateCalls != 1 { t.Fatalf("preparation calls input=%d check=%d state=%d", inputCalls, checkCalls, stateCalls) @@ -111,53 +111,65 @@ func TestComparisonPreparesEqualSelectionsWithSeparateTypedInputs(t *testing.T) } } -func TestComparisonPlanStoresPairSeparatelyFromOrdinaryInputs(t *testing.T) { +func TestComparisonProviderReceivesCheckedPairAndSharedState(t *testing.T) { type input struct{ ID int } - inputCalls, checkCalls, stateCalls := 0, 0, 0 - prepared := captureSuite(Suite[input, struct{}]{ + type state struct{ Label string } + inputCalls, checkCalls, stateCalls, compareCalls, acquireCalls := 0, 0, 0, 0, 0 + checked := make(map[Selection]input) + shared := &state{Label: "shared"} + var request ComparisonRequest[input] + app := newConfigTestApp(t, Command(Suite[input, *state]{ NewInput: func() input { inputCalls++ return input{ID: inputCalls} }, - NewState: func() struct{} { stateCalls++; return struct{}{} }, + NewState: func() *state { + stateCalls++ + if checkCalls != 2 { + t.Errorf("shared state created after %d input checks", checkCalls) + } + return shared + }, CheckInput: func(scope string, selection Selection, value input) error { checkCalls++ - if scope != "app" || value.ID != checkCalls { - return errors.New("comparison inputs were not prepared in order") + if scope != "app" { + return errors.New("unexpected scope") } + checked[selection] = value return nil }, - Scopes: []Scope[input, struct{}]{{ - Name: "app", Operations: []Operation{Diff}, - Compare: func(context.Context, ComparisonRequest[input], struct{}) (commandkit.Acquired[[]configkit.Difference], error) { + Scopes: []Scope[input, *state]{{ + Name: "app", Operations: []Operation{Show, Diff}, + Acquire: func(context.Context, Request, input, *state) (commandkit.Acquired[View], error) { + acquireCalls++ + return commandkit.Acquired[View]{}, errors.New("ordinary acquisition reached during comparison") + }, + Compare: func(_ context.Context, pair ComparisonRequest[input], value *state) (commandkit.Acquired[[]configkit.Difference], error) { + compareCalls++ + if checkCalls != 2 || stateCalls != 1 || value != shared { + t.Errorf("provider reached with checks=%d states=%d shared=%t", checkCalls, stateCalls, value == shared) + } + request = pair return commandkit.Acquired[[]configkit.Difference]{}, nil }, }}, - }) + })) + outcome := app.Execute(context.Background(), []string{ + "config", "diff", "--config", "before.yml", "--against-overlay=", + }, commandkit.Streams{Out: io.Discard, Err: io.Discard}) + if outcome.Kind != commandkit.OutcomeSuccess { + t.Fatalf("comparison outcome = %#v", outcome) + } left := Selection{Config: commandkit.Supplied[string]{Value: "before.yml", Set: true}} - right := Selection{Overlay: commandkit.Supplied[string]{Set: true}} - commandInput := &configCommandInput[input, struct{}]{ - selection: left, - against: right, - format: commandkit.Supplied[string]{Value: "yaml"}, + right := Selection{Config: left.Config, Overlay: commandkit.Supplied[string]{Set: true}} + if request.Left.Selection != left || request.Right.Selection != right { + t.Fatalf("provider selections = %#v", request) } - if err := prepareConfigOperation(prepared, Diff, commandInput); err != nil { - t.Fatal(err) + if len(checked) != 2 || request.Left.Input != checked[left] || request.Right.Input != checked[right] { + t.Fatalf("provider inputs = %#v, checked = %#v", request, checked) } - plan := commandInput.plan - if plan == nil || plan.comparison == nil || len(plan.scopes) != 0 { - t.Fatalf("comparison was not stored separately: %#v", plan) - } - if plan.comparison.scope.scope.Name != "app" { - t.Fatalf("comparison scope = %q", plan.comparison.scope.scope.Name) - } - wantRight := Selection{Config: left.Config, Overlay: right.Overlay} - request := plan.comparison.request - if request.Left.Selection != left || request.Right.Selection != wantRight || request.Left.Input.ID != 1 || request.Right.Input.ID != 2 { - t.Fatalf("prepared request = %#v", request) - } - if inputCalls != 2 || checkCalls != 2 || stateCalls != 0 { - t.Fatalf("preparation calls input=%d check=%d state=%d", inputCalls, checkCalls, stateCalls) + if inputCalls != 2 || checkCalls != 2 || stateCalls != 1 || compareCalls != 1 || acquireCalls != 0 { + t.Fatalf("invocation calls input=%d check=%d state=%d compare=%d acquire=%d", inputCalls, checkCalls, stateCalls, compareCalls, acquireCalls) } }