Skip to content

Commit 1f76d07

Browse files
joshua-templejoshua-temple
committed
test: cover directChildToward fully and pin settleParallelDone done-error
Raise proven coverage on two soundness-relevant kernel paths in state flagged by the v1.0 gap analysis. directChildToward (history.go:177): 55.6% -> 100%. A package-internal white-box test pins all three arms directly: the reachable spine descent (idx>0) and the two defensive not-found arms — leaf == compound (idx==0) and leaf not descending from the compound. Those arms are unreachable through the public Fire path (the sole caller recordHistory only ever passes a strict, isDescendant-filtered active leaf), so they are covered here as kernel invariants rather than contorted into a black-box scenario. settleParallelDone (parallel.go:523): 62.5% -> 68.8%. A black-box test drives the OnDone-action error arm: when a parallel completes and its OnDone action fails, Fire surfaces a typed ActionFailedError tagged onDone:<parallel> with OutcomeEffectError instead of swallowing it. The settleParallelDone upward-cascade arm (a parallel nested under a compound) surfaced a real soundness bug: the enclosing compound's OnDone is silently dropped because the cascade calls settleDone, which gates on IsFinal, and a parallel is never IsFinal. A nested compound reaching final propagates correctly, so this is an asymmetry, not intended behavior. The PIN encoding the correct expectation is committed as a skipped test with a loud reason so it activates the moment the bug is fixed; it is deliberately not weakened to the buggy zero-count. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
1 parent 64afa74 commit 1f76d07

2 files changed

Lines changed: 264 additions & 0 deletions

File tree

‎state/history_directchild_internal_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package state
2+
3+
import "testing"
4+
5+
// directChildToward (history.go:177) is the spine-descent helper recordHistory
6+
// uses to pick the compound's direct child toward a remembered leaf. Its two
7+
// not-found arms — leaf == compound (idx==0) and leaf not descending from compound
8+
// — are DEFENSIVE: the sole caller, recordHistory, only ever passes deep[0], a
9+
// proper active leaf already filtered to be a strict descendant of the compound
10+
// (recordHistory skips non-compounds and isDescendant-filters the leaf set, and a
11+
// compound is never itself a live leaf). The public Fire path therefore never
12+
// reaches those arms; the deep/shallow history tests in history_test.go exercise
13+
// only the happy descent (the idx>0 return).
14+
//
15+
// This white-box test pins all three arms directly so the defensive returns are
16+
// proven-correct rather than untested. It is package-internal because
17+
// directChildToward is an unexported method; the conscious, documented reason is
18+
// that the not-found arms are unreachable through the public API and must be
19+
// covered as kernel invariants here, not contorted into a black-box scenario.
20+
//
21+
// Topology (compound "root" with two subtrees):
22+
//
23+
// root
24+
// ├── a (compound)
25+
// │ └── a1 (leaf)
26+
// └── b (leaf)
27+
func directChildTowardMachine(t *testing.T) *Machine[string, string, struct{}] {
28+
t.Helper()
29+
return Forge[string, string, struct{}]("directchild").
30+
SuperState("root").
31+
Initial("a").
32+
SuperState("a").
33+
Initial("a1").
34+
SubState("a1").
35+
EndSuperState(). // close a
36+
SubState("b").
37+
EndSuperState(). // close root
38+
Initial("root").
39+
CurrentStateFn(func(struct{}) string { return "a1" }).
40+
Quench()
41+
}
42+
43+
func TestDirectChildToward_Branches(t *testing.T) {
44+
m := directChildTowardMachine(t)
45+
46+
t.Run("descends to direct child toward a nested leaf", func(t *testing.T) {
47+
// a1 is two levels below root (root -> a -> a1); the direct child of root
48+
// on a1's spine is "a". This is the reachable happy arm (idx>0).
49+
got, ok := m.directChildToward("root", "a1")
50+
if !ok {
51+
t.Fatalf("directChildToward(root, a1) ok=false, want true")
52+
}
53+
if got != "a" {
54+
t.Fatalf("directChildToward(root, a1) = %q, want %q (direct child on the spine)", got, "a")
55+
}
56+
})
57+
58+
t.Run("direct child that is itself the leaf", func(t *testing.T) {
59+
// b is a direct child leaf of root; directChildToward must return b itself.
60+
got, ok := m.directChildToward("root", "b")
61+
if !ok || got != "b" {
62+
t.Fatalf("directChildToward(root, b) = (%q,%v), want (b,true)", got, ok)
63+
}
64+
})
65+
66+
t.Run("DEFENSIVE: leaf == compound yields no proper descendant", func(t *testing.T) {
67+
// idx==0 arm (history.go:181): the compound is its own ancestor head, so
68+
// there is no proper descendant to descend toward. recordHistory never hits
69+
// this (a compound is never an active leaf), but the invariant is pinned.
70+
got, ok := m.directChildToward("root", "root")
71+
if ok {
72+
t.Fatalf("directChildToward(root, root) ok=true (got %q), want false: a compound has no proper descendant toward itself", got)
73+
}
74+
if got != "" {
75+
t.Fatalf("directChildToward(root, root) returned %q, want zero value", got)
76+
}
77+
})
78+
79+
t.Run("DEFENSIVE: leaf not descending from compound", func(t *testing.T) {
80+
// not-found arm (history.go:189): "b" does not descend from "a", so the
81+
// loop never finds "a" in b's ancestor chain. recordHistory never hits this
82+
// (it isDescendant-filters first), but the invariant is pinned.
83+
got, ok := m.directChildToward("a", "b")
84+
if ok {
85+
t.Fatalf("directChildToward(a, b) ok=true (got %q), want false: b does not descend from a", got)
86+
}
87+
if got != "" {
88+
t.Fatalf("directChildToward(a, b) returned %q, want zero value", got)
89+
}
90+
})
91+
92+
t.Run("DEFENSIVE: leaf unknown to the machine", func(t *testing.T) {
93+
// An unknown leaf has no chain containing the compound; the not-found arm
94+
// returns (zero,false) rather than panicking.
95+
got, ok := m.directChildToward("root", "ghost")
96+
if ok || got != "" {
97+
t.Fatalf("directChildToward(root, ghost) = (%q,%v), want (\"\",false)", got, ok)
98+
}
99+
})
100+
}

‎state/parallel_done_cascade_test.go‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
package state_test
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
"github.com/stablekernel/crucible/state"
9+
)
10+
11+
// This file raises proven coverage on settleParallelDone (parallel.go:521) by
12+
// driving, through the public Fire path, the two branches the v1.0 gap analysis
13+
// flagged as untested:
14+
//
15+
// - the OnDone-action ERROR branch: the parallel's own OnDone action fails, so
16+
// settleParallelDone must surface a typed *ActionFailedError tagged
17+
// "onDone:<parallel>" with OutcomeEffectError, rather than swallowing it.
18+
// - the upward-cascade branch: a parallel state nested *inside* a parent
19+
// compound, so once the parallel completes settleParallelDone runs and then
20+
// attempts to settle the parent's done (the `pn.hasParent` arm).
21+
//
22+
// Both branches are exercised end-to-end (no white-box reach-in) so the pins also
23+
// lock the observable Fire contract callers depend on.
24+
25+
// TestParallel_OnDoneActionError_SurfacesTaggedFailure drives the error arm of
26+
// settleParallelDone: when every region reaches final in one macrostep and the
27+
// parallel's OnDone action returns an error, Fire must report a typed
28+
// *ActionFailedError whose TransitionName identifies the parallel's onDone and
29+
// whose cause unwraps to the original error, with OutcomeEffectError. A swallowed
30+
// done-error would be a soundness hole: the caller would believe the macrostep
31+
// succeeded while the OnDone effect never ran.
32+
func TestParallel_OnDoneActionError_SurfacesTaggedFailure(t *testing.T) {
33+
boom := errors.New("ondone-boom")
34+
m := state.Forge[string, string, prCtx]("par-ondone-error").
35+
Action("failDone", func(state.ActionCtx[prCtx]) (state.Effect, error) {
36+
return nil, boom
37+
}).
38+
State("off").
39+
Transition("off").On("go").GoTo("par").
40+
SuperState("par").OnDone("failDone").
41+
Region("a").Initial("a1").SubState("a1").SubState("af").Final().EndRegion().
42+
Region("b").Initial("b1").SubState("b1").SubState("bf").Final().EndRegion().
43+
EndSuperState().
44+
Initial("off").
45+
CurrentStateFn(func(prCtx) string { return "off" }).
46+
Transition("a1").On("e").GoTo("af").
47+
Transition("b1").On("e").GoTo("bf").
48+
Quench()
49+
50+
inst := m.Cast(prCtx{}, state.WithInitialState("off"))
51+
ctx := context.Background()
52+
if res := inst.Fire(ctx, "go"); res.Err != nil {
53+
t.Fatalf("entering parallel: %v", res.Err)
54+
}
55+
56+
// Both regions reach final on the same event; the parallel's OnDone then runs
57+
// and fails.
58+
res := inst.Fire(ctx, "e")
59+
60+
var af *state.ActionFailedError
61+
if !errors.As(res.Err, &af) {
62+
t.Fatalf("err = %v, want *ActionFailedError", res.Err)
63+
}
64+
if !errors.Is(res.Err, boom) {
65+
t.Fatalf("err does not unwrap to boom: %v", res.Err)
66+
}
67+
if af.ActionName != "failDone" {
68+
t.Fatalf("ActionName = %q, want %q", af.ActionName, "failDone")
69+
}
70+
if af.TransitionName != "onDone:par" {
71+
t.Fatalf("TransitionName = %q, want %q (the parallel's onDone tag)", af.TransitionName, "onDone:par")
72+
}
73+
if res.Trace.Outcome != state.OutcomeEffectError {
74+
t.Fatalf("outcome = %v, want OutcomeEffectError", res.Trace.Outcome)
75+
}
76+
}
77+
78+
// TestParallel_NestedUnderCompound_SettlesParentDone drives the upward-cascade arm
79+
// of settleParallelDone (`if pn.hasParent { settleDone(...) }`): a parallel "par"
80+
// is the sole child of a compound "outer". When both regions of "par" reach final
81+
// in one macrostep, settleParallelDone runs par.OnDone and then attempts to settle
82+
// "outer"'s done — so outer.OnDone must fire too, and exactly once.
83+
//
84+
// par.OnDone emits "Pdone"; outer.OnDone emits "Odone". After the completing event
85+
// the effect stream must contain BOTH, in inner-then-outer order, each exactly
86+
// once.
87+
//
88+
// FREEZE BLOCKER (BUG): this test is SKIPPED because it currently FAILS — it has
89+
// surfaced a real soundness bug in settleParallelDone's upward cascade. When a
90+
// parallel completes inside a compound, outer.OnDone is silently dropped (0 times,
91+
// want 1), even though stateComplete(outer) is true. The cascade arm calls
92+
// i.settleDone(parallel, ...), but settleDone guards on n.state.IsFinal and a
93+
// PARALLEL state is never IsFinal, so the call is a no-op. The sibling helper
94+
// settleInteriorDone correctly gates on stateComplete(parent) instead. A nested
95+
// *compound* reaching final DOES propagate (verified manually), so this is an
96+
// asymmetry, not intended behavior. The PIN is left in place (Skip, not deleted)
97+
// so it activates the moment the bug is fixed; it must NOT be deleted or weakened
98+
// to the buggy 0-count. See the agent return for full reproduction.
99+
func TestParallel_NestedUnderCompound_SettlesParentDone(t *testing.T) {
100+
t.Skip("FREEZE BLOCKER: settleParallelDone upward cascade drops the enclosing compound's OnDone " +
101+
"(settleDone gates on IsFinal, parallels are never IsFinal). Un-skip once fixed.")
102+
103+
note := func(s string) state.ActionFn[prCtx] {
104+
return func(state.ActionCtx[prCtx]) (state.Effect, error) { return s, nil }
105+
}
106+
m := state.Forge[string, string, prCtx]("par-nested-done").
107+
Action("Pdone", note("Pdone")).
108+
Action("Odone", note("Odone")).
109+
State("off").
110+
Transition("off").On("go").GoTo("par").
111+
SuperState("outer").OnDone("Odone").
112+
Initial("par").
113+
SuperState("par").OnDone("Pdone").
114+
Region("a").Initial("a1").SubState("a1").SubState("af").Final().EndRegion().
115+
Region("b").Initial("b1").SubState("b1").SubState("bf").Final().EndRegion().
116+
EndSuperState(). // close par
117+
EndSuperState(). // close outer
118+
Initial("off").
119+
CurrentStateFn(func(prCtx) string { return "off" }).
120+
Transition("a1").On("e").GoTo("af").
121+
Transition("b1").On("e").GoTo("bf").
122+
Quench()
123+
124+
inst := m.Cast(prCtx{}, state.WithInitialState("off"))
125+
ctx := context.Background()
126+
if res := inst.Fire(ctx, "go"); res.Err != nil {
127+
t.Fatalf("entering nested parallel: %v", res.Err)
128+
}
129+
130+
res := inst.Fire(ctx, "e")
131+
if res.Err != nil {
132+
t.Fatalf("e errored: %v (config=%v)", res.Err, inst.Configuration())
133+
}
134+
135+
// par.OnDone must fire exactly once.
136+
if got := countEffect(res.Effects, "Pdone"); got != 1 {
137+
t.Fatalf("par.OnDone fired %d times, want exactly 1 (effects=%v)",
138+
got, stringEffects(res.Effects))
139+
}
140+
141+
// The cascade-upward arm: once "par" completes, its enclosing compound "outer"
142+
// is also complete (par is outer's only active descendant and is done), so
143+
// outer.OnDone must fire — exactly once.
144+
if got := countEffect(res.Effects, "Odone"); got != 1 {
145+
t.Fatalf("outer.OnDone fired %d times after the nested parallel completed, want exactly 1 (effects=%v)\n"+
146+
"the settleParallelDone upward-cascade arm must propagate done to the enclosing compound",
147+
got, stringEffects(res.Effects))
148+
}
149+
150+
// Inner-then-outer ordering: the parallel's own done settles before its parent's.
151+
got := stringEffects(res.Effects)
152+
pIdx, oIdx := -1, -1
153+
for i, e := range got {
154+
switch e {
155+
case "Pdone":
156+
pIdx = i
157+
case "Odone":
158+
oIdx = i
159+
}
160+
}
161+
if pIdx < 0 || oIdx < 0 || pIdx >= oIdx {
162+
t.Fatalf("effect order = %v, want Pdone before Odone (inner done settles before the enclosing compound's done)", got)
163+
}
164+
}

0 commit comments

Comments
 (0)