Skip to content

Commit f03df6a

Browse files
committed
test(recipes): assert recorded-ness separately from the recorded mode
Review on #2328: comparing the mode alone conflated two states. An absent configuration and a configuration that is present but carries an empty mode both yield "", so a regression producing the latter would have passed as the former. That is the ambiguous-presence-check anti-pattern the repo guidance calls out. Switched to RuntimeInventoryMode()'s two return values and assert presence and value separately. Mutation-verified: making the accessor report an absent configuration as present fails the new assertion. Worth noting the present-but-empty state is not reachable through the public API today, since the only writer sets Mode from a validated parse. This guards a future regression rather than a current defect. Signed-off-by: Mark Chmarny <mark@chmarny.com>
1 parent 0162201 commit f03df6a

1 file changed

Lines changed: 18 additions & 11 deletions

File tree

pkg/recipe/k8s_aibom_stock_adoption_test.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,19 @@ func TestK8sAIBOMStockAdoption(t *testing.T) {
5757
opts []recipe.BuildOption
5858
wantDeclared bool
5959
wantEnabled bool
60-
// wantMode is the selection the emitted recipe must record. Empty
61-
// means nothing should be recorded, which is the case for every
62-
// build that does not pass the flag. Asserted separately from
63-
// IsEnabled because a regression that declines the component while
64-
// dropping the recorded decision would otherwise pass: ADR-019
65-
// section E requires the recipe to carry the decision, not just its
66-
// effect.
67-
wantMode recipe.RuntimeInventoryMode
60+
// wantRecorded and wantMode are asserted separately, against
61+
// RuntimeInventoryMode()'s two return values. Comparing the mode
62+
// alone would collapse two distinct states into "": no configuration
63+
// recorded at all, which is correct for a build that does not pass
64+
// the flag, and a configuration that is present but carries an empty
65+
// mode, which is invalid. A regression producing the latter must not
66+
// pass as the former.
67+
//
68+
// Asserted at all because ADR-019 section E requires the recipe to
69+
// carry the decision, not just its effect, so a build that declines
70+
// the component while dropping the record is still a regression.
71+
wantRecorded bool
72+
wantMode recipe.RuntimeInventoryMode
6873
}{
6974
{
7075
name: "target stock recipe declares and enables the component",
@@ -88,6 +93,7 @@ func TestK8sAIBOMStockAdoption(t *testing.T) {
8893
opts: []recipe.BuildOption{recipe.WithRuntimeInventoryMode(recipe.RuntimeInventoryDisabled)},
8994
wantDeclared: true,
9095
wantEnabled: false,
96+
wantRecorded: true,
9197
wantMode: recipe.RuntimeInventoryDisabled,
9298
},
9399
{
@@ -111,9 +117,10 @@ func TestK8sAIBOMStockAdoption(t *testing.T) {
111117
t.Fatalf("BuildFromCriteria() error = %v", err)
112118
}
113119

114-
var gotMode recipe.RuntimeInventoryMode
115-
if result.Configuration != nil && result.Configuration.RuntimeInventory != nil {
116-
gotMode = result.Configuration.RuntimeInventory.Mode
120+
gotMode, gotRecorded := result.RuntimeInventoryMode()
121+
if gotRecorded != tt.wantRecorded {
122+
t.Errorf("RuntimeInventoryMode() recorded = %v, want %v: an absent configuration and a present-but-empty one must not be conflated",
123+
gotRecorded, tt.wantRecorded)
117124
}
118125
if gotMode != tt.wantMode {
119126
t.Errorf("configuration.runtimeInventory.mode = %q, want %q: the recipe must record the decision, not just its effect",

0 commit comments

Comments
 (0)