Skip to content

Commit 02a51e3

Browse files
committed
DRA ResourceSlice tracker: explain test a bit better, fix -run
Instead of creating a new test case, the permutation is passed down. This enables adding the event numbers to the log output, which is useful to understand better which output belongs to which input: === RUN TestListPatchedResourceSlices/update-patch/2_3_0_1 tracker.go:396: I0929 14:28:40.032318] event #1: ResourceSlice add slice="s1" tracker.go:581: I0929 14:28:40.032404] event #1: syncing ResourceSlice resourceslice="s1" tracker.go:659: I0929 14:28:40.032446] event #1: ResourceSlice synced resourceslice="s1" change="add" tracker.go:396: I0929 14:28:40.032502] event #2: ResourceSlice add slice="s2" tracker.go:581: I0929 14:28:40.032536] event #2: syncing ResourceSlice resourceslice="s2" tracker.go:659: I0929 14:28:40.032568] event #2: ResourceSlice synced resourceslice="s2" change="add" tracker.go:463: I0929 14:28:40.032609] event #0/#0: DeviceTaintRule add patch="rule" tracker.go:581: I0929 14:28:40.032639] event #0/#0: syncing ResourceSlice resourceslice="s1" tracker.go:703: I0929 14:28:40.032675] event #0/#0: processing DeviceTaintRule resourceslice="s1" deviceTaintRule="rule" tracker.go:807: I0929 14:28:40.032712] event #0/#0: applying matching DeviceTaintRule resourceslice="s1" deviceTaintRule="rule" device="driver1.example.com/pool-1/device-1" tracker.go:868: I0929 14:28:40.032780] event #0/#0: Assigned new taint ID, no matching taint resourceslice="s1" deviceTaintRule="rule" device="driver1.example.com/pool-1/device-1" taintID=0 taint="example.com/taint=tainted:NoExecute" tracker.go:654: I0929 14:28:40.033023] event #0/#0: ResourceSlice synced resourceslice="s1" change="update" diff=< @@ -23,7 +23,32 @@ "BindingConditions": null, "BindingFailureConditions": null, "AllowMultipleAllocations": null, - "Taints": null + "Taints": [ + { + "Rule": { + "metadata": { + "name": "rule" + }, + "spec": { + "deviceSelector": { + "pool": "pool-1" + }, + "taint": { + "key": "example.com/taint", + "value": "tainted", + "effect": "NoExecute", + "timeAdded": "2006-01-02T15:04:05Z" + } + }, + "status": {} + }, + "ID": 1, + "key": "example.com/taint", + "value": "tainted", + "effect": "NoExecute", + "timeAdded": "2006-01-02T15:04:05Z" + } + ] } ], "Taints": null, > tracker.go:482: I0929 14:28:40.033224] event #0/#1: DeviceTaintRule update patch="rule" diff=< @@ -4,7 +4,7 @@ }, "spec": { "deviceSelector": { - "pool": "pool-1" + "pool": "pool-2" }, "taint": { "key": "example.com/taint", > tracker.go:581: I0929 14:28:40.033285] event #0/#1: syncing ResourceSlice resourceslice="s1" tracker.go:703: I0929 14:28:40.033319] event #0/#1: processing DeviceTaintRule resourceslice="s1" deviceTaintRule="rule" tracker.go:654: I0929 14:28:40.033478] event #0/#1: ResourceSlice synced resourceslice="s1" change="update" diff=< @@ -23,32 +23,7 @@ "BindingConditions": null, "BindingFailureConditions": null, "AllowMultipleAllocations": null, - "Taints": [ - { - "Rule": { - "metadata": { - "name": "rule" - }, - "spec": { - "deviceSelector": { - "pool": "pool-1" - }, - "taint": { - "key": "example.com/taint", - "value": "tainted", - "effect": "NoExecute", - "timeAdded": "2006-01-02T15:04:05Z" - } - }, - "status": {} - }, - "ID": 1, - "key": "example.com/taint", - "value": "tainted", - "effect": "NoExecute", - "timeAdded": "2006-01-02T15:04:05Z" - } - ] + "Taints": null } ], "Taints": null, > tracker.go:581: I0929 14:28:40.033601] event #0/#1: syncing ResourceSlice resourceslice="s2" tracker.go:703: I0929 14:28:40.033633] event #0/#1: processing DeviceTaintRule resourceslice="s2" deviceTaintRule="rule" ... Disabling event checking only worked when actually running all sub-tests. When selectively running only one permutation with -run, the boolean variable was wrong: $ go test -run='.*/^update-patch$' ./staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/ ok k8s.io/dynamic-resource-allocation/resourceslice/tracker $ go test -run='.*/^update-patch$/3_2_0_1' ./staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/ --- FAIL: TestListPatchedResourceSlices (0.01s) --- FAIL: TestListPatchedResourceSlices/update-patch (0.00s) --- FAIL: TestListPatchedResourceSlices/update-patch/3_2_0_1 (0.00s) tracker_test.go:762: Error Trace: /nvme/gopath/src/k8s.io/kubernetes/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go:762 /nvme/gopath/src/k8s.io/kubernetes/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go:856 Error: Not equal: expected: []tracker.handlerEvent{tracker.handlerEvent{event:"add", oldObj:(*api.ResourceSlice)(nil), newObj:(*api.ResourceSlice)(0xc000301d40)}, tracker.handlerEvent{event:"add", oldObj:(*api.ResourceSlice)(nil), newObj:(*api.ResourceSlice)(0xc000346000)}} actual : []tracker.handlerEvent{tracker.handlerEvent{event:"add", oldObj:(*api.ResourceSlice)(nil), newObj:(*api.ResourceSlice)(0xc0001f9ba0)}, tracker.handlerEvent{event:"add", oldObj:(*api.ResourceSlice)(nil), newObj:(*api.ResourceSlice)(0xc000301d40)}, tracker.handlerEvent{event:"update", oldObj:(*api.ResourceSlice)(0xc000301d40), newObj:(*api.ResourceSlice)(0xc0003dba00)}, tracker.handlerEvent{event:"update", oldObj:(*api.ResourceSlice)(0xc0003dba00), newObj:(*api.ResourceSlice)(0xc000301d40)}, tracker.handlerEvent{event:"update", oldObj:(*api.ResourceSlice)(0xc0001f9ba0), newObj:(*api.ResourceSlice)(0xc0003dbba0)}} Now permutations are detected automatically based on the indices. While at it, documentation gets moved around a bit to make reading test cases easier without going to the implementation.
1 parent a8a21aa commit 02a51e3

File tree

1 file changed

+80
-47
lines changed
  • staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker

1 file changed

+80
-47
lines changed

staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go

Lines changed: 80 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,35 @@ func update[T any](oldObj, newObj *T) [2]*T {
6767
return [2]*T{oldObj, newObj}
6868
}
6969

70-
func runInputEvents(tCtx *testContext, events []any) {
71-
for _, event := range events {
72-
switch event := event.(type) {
73-
case []any:
74-
for _, event := range event {
75-
applyEventPair(tCtx, event)
70+
func runInputEvents(tCtx *testContext, events []any, permutation []int) {
71+
for _, i := range permutation {
72+
event, name := lookupEvent(events, i)
73+
stepCtx := tCtx.withLoggerName(fmt.Sprintf("event #%s", name))
74+
applyEventPair(stepCtx, event)
75+
}
76+
}
77+
78+
// lookupEvent is the opposite of flatten: it takes an index
79+
// after flattening and maps it back to the event in the original
80+
// event hierarchy. To do so, it descends into the second level where necessary.
81+
func lookupEvent(events []any, index int) (any, string) {
82+
numEvents := 0
83+
for i := range events {
84+
if e, ok := events[i].([]any); ok {
85+
for j := range e {
86+
if numEvents == index {
87+
return e[j], fmt.Sprintf("%d/%d", i, j)
88+
}
89+
numEvents++
7690
}
77-
default:
78-
applyEventPair(tCtx, event)
91+
} else {
92+
if numEvents == index {
93+
return events[i], fmt.Sprintf("%d", i)
94+
}
95+
numEvents++
7996
}
8097
}
98+
panic(fmt.Sprintf("invalid event index #%d", index))
8199
}
82100

83101
func applyEventPair(tCtx *testContext, event any) {
@@ -140,6 +158,18 @@ type testContext struct {
140158
*fake.Clientset
141159
}
142160

161+
func (t *testContext) withLoggerName(name string) *testContext {
162+
logger := klog.FromContext(t.Context)
163+
logger = klog.LoggerWithName(logger, name)
164+
t = &testContext{
165+
T: t.T,
166+
Context: klog.NewContext(t.Context, logger),
167+
Tracker: t.Tracker,
168+
Clientset: t.Clientset,
169+
}
170+
return t
171+
}
172+
143173
var (
144174
now, _ = time.Parse(time.RFC3339, "2006-01-02T15:04:05Z")
145175
driver1 = "driver1.example.com"
@@ -335,6 +365,11 @@ func TestListPatchedResourceSlices(t *testing.T) {
335365
// the order in those nested lists is preserved.
336366
events []any
337367
expectedPatchedSlices []*resourceapi.ResourceSlice
368+
// The exact events that are emitted for a sequence of events is
369+
// highly dependent on the order in which those events are received.
370+
// We punt on determining a set of validation criteria for every
371+
// possible sequence and only check them against the first
372+
// permutation: the order in which the events are defined.
338373
expectedHandlerEvents []handlerEvent
339374
expectEvents func(t *assert.CollectT, events *v1.EventList)
340375
expectUnhandledErrors func(t *testing.T, errs []error)
@@ -643,7 +678,15 @@ func TestListPatchedResourceSlices(t *testing.T) {
643678
}
644679
}
645680

646-
testHandlers := func(tCtx *testContext, test test, testExpectedEmittedEvents bool) {
681+
testHandlers := func(tCtx *testContext, test test, permutation []int) {
682+
isPermutated := false
683+
for i, j := range permutation {
684+
if i != j {
685+
isPermutated = true
686+
break
687+
}
688+
}
689+
647690
var handlerEvents []handlerEvent
648691
handler := cache.ResourceEventHandlerFuncs{
649692
AddFunc: func(obj interface{}) {
@@ -663,9 +706,9 @@ func TestListPatchedResourceSlices(t *testing.T) {
663706
unhandledErrors = append(unhandledErrors, err)
664707
}
665708

666-
runInputEvents(tCtx, test.events)
709+
runInputEvents(tCtx, test.events, permutation)
667710

668-
if testExpectedEmittedEvents {
711+
if !isPermutated {
669712
assert.Equal(tCtx, test.expectedHandlerEvents, handlerEvents)
670713
}
671714

@@ -709,60 +752,47 @@ func TestListPatchedResourceSlices(t *testing.T) {
709752

710753
for name, tc := range tests {
711754
t.Run(name, func(t *testing.T) {
712-
// The exact events that are emitted for a sequence of events is
713-
// highly dependent on the order in which those events are received.
714-
// We punt on determining a set of validation criteria for every
715-
// possible sequence and only check them against the first
716-
// permutation: the order in which the events are defined.
717-
testExpectedEmittedEvents := true
718-
719-
numEvents := len(tc.events)
720-
if numEvents <= 1 {
721-
// No permutations.
722-
tContext := setup(t)
723-
testHandlers(tContext, tc, testExpectedEmittedEvents)
724-
return
725-
}
726-
727-
// flatten does one level of flattening of events. It also returns
728-
// another slice of pairs of indices representing ranges which were
729-
// flattened.
730-
flatten := func(events []any) ([]any, [][2]int) {
731-
var ret []any
755+
// flatten does one level of flattening of events, counting all events.
756+
// It also returns a slice of pairs of indices representing ranges which were
757+
// flattened (= came from the second level) and which therefore must
758+
// remain in that order.
759+
flatten := func(events []any) (int, [][2]int) {
760+
numEvents := 0
732761
var ranges [][2]int
733762
for _, e := range events {
734763
switch e := e.(type) {
735764
case []any:
736-
ranges = append(ranges, [2]int{len(ret), len(ret) + len(e)})
737-
ret = append(ret, e...)
765+
ranges = append(ranges, [2]int{numEvents, numEvents + len(e)})
766+
numEvents += len(e)
738767
default:
739-
ret = append(ret, e)
768+
numEvents++
740769
}
741770
}
742-
return ret, ranges
771+
return numEvents, ranges
743772
}
773+
numEvents, constraints := flatten(tc.events)
744774

745-
var constraints [][2]int
746-
tc.events, constraints = flatten(tc.events)
747-
numEvents = len(tc.events)
775+
if len(tc.events) <= 1 {
776+
// No permutations possible.
777+
var permutation []int
778+
for i := 0; i < numEvents; i++ {
779+
permutation = append(permutation, i)
780+
}
781+
tContext := setup(t)
782+
testHandlers(tContext, tc, permutation)
783+
return
784+
}
748785

749786
permutation := make([]int, numEvents)
750787
var permutate func(depth int)
751788
permutate = func(depth int) {
752789
if depth >= numEvents {
753790
// Define a sub-test which runs the current permutation of events.
754-
events := make([]any, numEvents)
755-
for i := range numEvents {
756-
events[i] = tc.events[permutation[i]]
757-
}
758-
tc := tc
759-
tc.events = events
760791
name := strings.Trim(fmt.Sprintf("%v", permutation), "[]")
761792
t.Run(name, func(t *testing.T) {
762793
tContext := setup(t)
763-
testHandlers(tContext, tc, testExpectedEmittedEvents)
764-
765-
testExpectedEmittedEvents = false
794+
// No need to clone the slice, we don't run in parallel.
795+
testHandlers(tContext, tc, permutation)
766796
})
767797
return
768798
}
@@ -778,10 +808,13 @@ func TestListPatchedResourceSlices(t *testing.T) {
778808
}
779809
for j := i + 1; j < constraint[1]; j++ {
780810
if slices.Contains(permutation[0:depth], j) {
811+
// Invalid permutation, would change order
812+
// of sub-events.
781813
continue nexti
782814
}
783815
}
784816
}
817+
785818
// Pick it for the current position in permutation,
786819
// continue with next position.
787820
permutation[depth] = i

0 commit comments

Comments
 (0)