Skip to content

Commit 8f5d0e0

Browse files
committed
keyviz: HistoryColumns accessor + plug withFlags coverage gap
Round-2 review fix for PR #651 (Claude bot minor): withFlags helper saved/restored every keyviz flag except the new --keyvizHistoryColumns, so a future test mutating the flag through the helper would leak state across parallel tests. Add the slot, update both call sites, and pin the flag → buildKeyVizSampler → MemSamplerOptions.HistoryColumns forwarding by asserting s.HistoryColumns() in TestBuildKeyVizSamplerHonorsEnabledFlag. Also adds a HistoryColumns() accessor on *MemSampler paralleling Step() so wiring tests don't need to reach into the unexported opts struct.
1 parent 5bddeba commit 8f5d0e0

2 files changed

Lines changed: 22 additions & 4 deletions

File tree

keyviz/sampler.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,17 @@ func (s *MemSampler) Step() time.Duration {
774774
return s.opts.Step
775775
}
776776

777+
// HistoryColumns returns the configured ring-buffer length after
778+
// applying defaults and the MaxHistoryColumns clamp. Wiring tests use
779+
// this to verify --keyvizHistoryColumns is forwarded end-to-end
780+
// without exposing the internal opts struct.
781+
func (s *MemSampler) HistoryColumns() int {
782+
if s == nil {
783+
return DefaultHistoryColumns
784+
}
785+
return s.opts.HistoryColumns
786+
}
787+
777788
// appendDrainedRow swaps the slot's counters to zero and appends a
778789
// MatrixRow when any counter was non-zero. Idle slots are skipped.
779790
// Metadata is read under the slot's metaMu so a concurrent

main_keyviz_test.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,20 @@ import (
1414
// TestBuildKeyVizSamplerHonorsEnabledFlag pins the on/off contract:
1515
// --keyvizEnabled=false returns nil (so coordinator/admin server take
1616
// the disabled paths), and --keyvizEnabled=true with explicit options
17-
// returns a configured sampler.
17+
// returns a configured sampler whose options match every flag.
1818
func TestBuildKeyVizSamplerHonorsEnabledFlag(t *testing.T) {
1919
t.Parallel()
20-
withFlags(t, false, time.Second, 5, 7, func() {
20+
withFlags(t, false, time.Second, 5, 7, 16, func() {
2121
require.Nil(t, buildKeyVizSampler())
2222
})
23-
withFlags(t, true, 250*time.Millisecond, 5, 7, func() {
23+
withFlags(t, true, 250*time.Millisecond, 5, 7, 16, func() {
2424
s := buildKeyVizSampler()
2525
require.NotNil(t, s)
2626
require.Equal(t, 250*time.Millisecond, s.Step())
27+
// Pin the --keyvizHistoryColumns forwarding so a future
28+
// refactor that drops the flag from buildKeyVizSampler is
29+
// caught here, not at runtime.
30+
require.Equal(t, 16, s.HistoryColumns())
2731
})
2832
}
2933

@@ -95,23 +99,26 @@ func withFlags(
9599
t *testing.T,
96100
enabled bool,
97101
step time.Duration,
98-
maxTracked, maxMembers int,
102+
maxTracked, maxMembers, historyColumns int,
99103
fn func(),
100104
) {
101105
t.Helper()
102106
prevEnabled := *keyvizEnabled
103107
prevStep := *keyvizStep
104108
prevMaxTracked := *keyvizMaxTrackedRoutes
105109
prevMaxMembers := *keyvizMaxMemberRoutesPerSlot
110+
prevHistoryColumns := *keyvizHistoryColumns
106111
*keyvizEnabled = enabled
107112
*keyvizStep = step
108113
*keyvizMaxTrackedRoutes = maxTracked
109114
*keyvizMaxMemberRoutesPerSlot = maxMembers
115+
*keyvizHistoryColumns = historyColumns
110116
defer func() {
111117
*keyvizEnabled = prevEnabled
112118
*keyvizStep = prevStep
113119
*keyvizMaxTrackedRoutes = prevMaxTracked
114120
*keyvizMaxMemberRoutesPerSlot = prevMaxMembers
121+
*keyvizHistoryColumns = prevHistoryColumns
115122
}()
116123
fn()
117124
}

0 commit comments

Comments
 (0)