From 383ce85649cc9f4c59a2ecbe695444941ea4c0d1 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Tue, 25 May 2021 09:21:08 -0400 Subject: [PATCH] [scheduler] avoid comparing function pointers in unit tests PluginFactory is a function that returns a plugin. We have been comparing these functions in unit tests and it has worked so far, but starts to fail in gotip/master. Note from the golang team: ``` Func values are incomparable. It is true that you could get the PC through reflection but it is still not expected to be compared. PCs can change due to inlining, wrappers, etc., depending on the compiler's details, which is subject to change. ``` Signed-off-by: Davanum Srinivas --- .../framework/runtime/registry_test.go | 51 ++++++++++++------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/pkg/scheduler/framework/runtime/registry_test.go b/pkg/scheduler/framework/runtime/registry_test.go index f9bc363c11f..9ace42394b1 100644 --- a/pkg/scheduler/framework/runtime/registry_test.go +++ b/pkg/scheduler/framework/runtime/registry_test.go @@ -20,6 +20,8 @@ import ( "reflect" "testing" + "github.com/google/uuid" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/scheduler/framework" ) @@ -76,7 +78,9 @@ func TestDecodeInto(t *testing.T) { func isRegistryEqual(registryX, registryY Registry) bool { for name, pluginFactory := range registryY { if val, ok := registryX[name]; ok { - if reflect.ValueOf(pluginFactory).Pointer() != reflect.ValueOf(val).Pointer() { + p1, _ := pluginFactory(nil, nil) + p2, _ := val(nil, nil) + if p1.Name() != p2.Name() { // pluginFactory functions are not the same. return false } @@ -96,19 +100,24 @@ func isRegistryEqual(registryX, registryY Registry) bool { return true } -type mockNoopPlugin struct{} +type mockNoopPlugin struct { + uuid string +} func (p *mockNoopPlugin) Name() string { - return "MockNoop" + return p.uuid } func NewMockNoopPluginFactory() PluginFactory { + uuid := uuid.New().String() return func(_ runtime.Object, _ framework.Handle) (framework.Plugin, error) { - return &mockNoopPlugin{}, nil + return &mockNoopPlugin{uuid}, nil } } func TestMerge(t *testing.T) { + m1 := NewMockNoopPluginFactory() + m2 := NewMockNoopPluginFactory() tests := []struct { name string primaryRegistry Registry @@ -119,27 +128,27 @@ func TestMerge(t *testing.T) { { name: "valid Merge", primaryRegistry: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), + "pluginFactory1": m1, }, registryToMerge: Registry{ - "pluginFactory2": NewMockNoopPluginFactory(), + "pluginFactory2": m2, }, expected: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), - "pluginFactory2": NewMockNoopPluginFactory(), + "pluginFactory1": m1, + "pluginFactory2": m2, }, shouldError: false, }, { name: "Merge duplicate factories", primaryRegistry: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), + "pluginFactory1": m1, }, registryToMerge: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), + "pluginFactory1": m2, }, expected: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), + "pluginFactory1": m1, }, shouldError: true, }, @@ -162,6 +171,8 @@ func TestMerge(t *testing.T) { } func TestRegister(t *testing.T) { + m1 := NewMockNoopPluginFactory() + m2 := NewMockNoopPluginFactory() tests := []struct { name string registry Registry @@ -174,21 +185,21 @@ func TestRegister(t *testing.T) { name: "valid Register", registry: Registry{}, nameToRegister: "pluginFactory1", - factoryToRegister: NewMockNoopPluginFactory(), + factoryToRegister: m1, expected: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), + "pluginFactory1": m1, }, shouldError: false, }, { name: "Register duplicate factories", registry: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), + "pluginFactory1": m1, }, nameToRegister: "pluginFactory1", - factoryToRegister: NewMockNoopPluginFactory(), + factoryToRegister: m2, expected: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), + "pluginFactory1": m1, }, shouldError: true, }, @@ -211,6 +222,8 @@ func TestRegister(t *testing.T) { } func TestUnregister(t *testing.T) { + m1 := NewMockNoopPluginFactory() + m2 := NewMockNoopPluginFactory() tests := []struct { name string registry Registry @@ -221,12 +234,12 @@ func TestUnregister(t *testing.T) { { name: "valid Unregister", registry: Registry{ - "pluginFactory1": NewMockNoopPluginFactory(), - "pluginFactory2": NewMockNoopPluginFactory(), + "pluginFactory1": m1, + "pluginFactory2": m2, }, nameToUnregister: "pluginFactory1", expected: Registry{ - "pluginFactory2": NewMockNoopPluginFactory(), + "pluginFactory2": m2, }, shouldError: false, },