Test comparison preparation through provider behavior
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user