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.
This commit is contained in:
Patrick Ohly
2025-09-29 14:30:07 +02:00
parent a8a21aaf85
commit 02a51e3712

View File

@@ -67,17 +67,35 @@ func update[T any](oldObj, newObj *T) [2]*T {
return [2]*T{oldObj, newObj}
}
func runInputEvents(tCtx *testContext, events []any) {
for _, event := range events {
switch event := event.(type) {
case []any:
for _, event := range event {
applyEventPair(tCtx, event)
func runInputEvents(tCtx *testContext, events []any, permutation []int) {
for _, i := range permutation {
event, name := lookupEvent(events, i)
stepCtx := tCtx.withLoggerName(fmt.Sprintf("event #%s", name))
applyEventPair(stepCtx, event)
}
}
// lookupEvent is the opposite of flatten: it takes an index
// after flattening and maps it back to the event in the original
// event hierarchy. To do so, it descends into the second level where necessary.
func lookupEvent(events []any, index int) (any, string) {
numEvents := 0
for i := range events {
if e, ok := events[i].([]any); ok {
for j := range e {
if numEvents == index {
return e[j], fmt.Sprintf("%d/%d", i, j)
}
numEvents++
}
default:
applyEventPair(tCtx, event)
} else {
if numEvents == index {
return events[i], fmt.Sprintf("%d", i)
}
numEvents++
}
}
panic(fmt.Sprintf("invalid event index #%d", index))
}
func applyEventPair(tCtx *testContext, event any) {
@@ -140,6 +158,18 @@ type testContext struct {
*fake.Clientset
}
func (t *testContext) withLoggerName(name string) *testContext {
logger := klog.FromContext(t.Context)
logger = klog.LoggerWithName(logger, name)
t = &testContext{
T: t.T,
Context: klog.NewContext(t.Context, logger),
Tracker: t.Tracker,
Clientset: t.Clientset,
}
return t
}
var (
now, _ = time.Parse(time.RFC3339, "2006-01-02T15:04:05Z")
driver1 = "driver1.example.com"
@@ -335,6 +365,11 @@ func TestListPatchedResourceSlices(t *testing.T) {
// the order in those nested lists is preserved.
events []any
expectedPatchedSlices []*resourceapi.ResourceSlice
// The exact events that are emitted for a sequence of events is
// highly dependent on the order in which those events are received.
// We punt on determining a set of validation criteria for every
// possible sequence and only check them against the first
// permutation: the order in which the events are defined.
expectedHandlerEvents []handlerEvent
expectEvents func(t *assert.CollectT, events *v1.EventList)
expectUnhandledErrors func(t *testing.T, errs []error)
@@ -643,7 +678,15 @@ func TestListPatchedResourceSlices(t *testing.T) {
}
}
testHandlers := func(tCtx *testContext, test test, testExpectedEmittedEvents bool) {
testHandlers := func(tCtx *testContext, test test, permutation []int) {
isPermutated := false
for i, j := range permutation {
if i != j {
isPermutated = true
break
}
}
var handlerEvents []handlerEvent
handler := cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) {
@@ -663,9 +706,9 @@ func TestListPatchedResourceSlices(t *testing.T) {
unhandledErrors = append(unhandledErrors, err)
}
runInputEvents(tCtx, test.events)
runInputEvents(tCtx, test.events, permutation)
if testExpectedEmittedEvents {
if !isPermutated {
assert.Equal(tCtx, test.expectedHandlerEvents, handlerEvents)
}
@@ -709,60 +752,47 @@ func TestListPatchedResourceSlices(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// The exact events that are emitted for a sequence of events is
// highly dependent on the order in which those events are received.
// We punt on determining a set of validation criteria for every
// possible sequence and only check them against the first
// permutation: the order in which the events are defined.
testExpectedEmittedEvents := true
numEvents := len(tc.events)
if numEvents <= 1 {
// No permutations.
tContext := setup(t)
testHandlers(tContext, tc, testExpectedEmittedEvents)
return
}
// flatten does one level of flattening of events. It also returns
// another slice of pairs of indices representing ranges which were
// flattened.
flatten := func(events []any) ([]any, [][2]int) {
var ret []any
// flatten does one level of flattening of events, counting all events.
// It also returns a slice of pairs of indices representing ranges which were
// flattened (= came from the second level) and which therefore must
// remain in that order.
flatten := func(events []any) (int, [][2]int) {
numEvents := 0
var ranges [][2]int
for _, e := range events {
switch e := e.(type) {
case []any:
ranges = append(ranges, [2]int{len(ret), len(ret) + len(e)})
ret = append(ret, e...)
ranges = append(ranges, [2]int{numEvents, numEvents + len(e)})
numEvents += len(e)
default:
ret = append(ret, e)
numEvents++
}
}
return ret, ranges
return numEvents, ranges
}
numEvents, constraints := flatten(tc.events)
var constraints [][2]int
tc.events, constraints = flatten(tc.events)
numEvents = len(tc.events)
if len(tc.events) <= 1 {
// No permutations possible.
var permutation []int
for i := 0; i < numEvents; i++ {
permutation = append(permutation, i)
}
tContext := setup(t)
testHandlers(tContext, tc, permutation)
return
}
permutation := make([]int, numEvents)
var permutate func(depth int)
permutate = func(depth int) {
if depth >= numEvents {
// Define a sub-test which runs the current permutation of events.
events := make([]any, numEvents)
for i := range numEvents {
events[i] = tc.events[permutation[i]]
}
tc := tc
tc.events = events
name := strings.Trim(fmt.Sprintf("%v", permutation), "[]")
t.Run(name, func(t *testing.T) {
tContext := setup(t)
testHandlers(tContext, tc, testExpectedEmittedEvents)
testExpectedEmittedEvents = false
// No need to clone the slice, we don't run in parallel.
testHandlers(tContext, tc, permutation)
})
return
}
@@ -778,10 +808,13 @@ func TestListPatchedResourceSlices(t *testing.T) {
}
for j := i + 1; j < constraint[1]; j++ {
if slices.Contains(permutation[0:depth], j) {
// Invalid permutation, would change order
// of sub-events.
continue nexti
}
}
}
// Pick it for the current position in permutation,
// continue with next position.
permutation[depth] = i