From 2d446d1e5d564a0d92c3a4d0cd16c7be915e2630 Mon Sep 17 00:00:00 2001 From: SP12893678 <36910625+SP12893678@users.noreply.github.com> Date: Wed, 17 Jul 2024 21:03:21 +0800 Subject: [PATCH 1/3] [YUNIKORN-2762] Improve util funtion's test coverage --- pkg/common/utils/utils.go | 7 ++- pkg/common/utils/utils_test.go | 98 ++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index 52eab0e51..eb9f71208 100644 --- a/pkg/common/utils/utils.go +++ b/pkg/common/utils/utils.go @@ -20,6 +20,7 @@ package utils import ( "encoding/json" + "errors" "fmt" "reflect" "strconv" @@ -44,6 +45,10 @@ const userInfoKey = siCommon.DomainYuniKorn + "user.info" const uniqueAutogenSuffix = "-uniqueautogen" var pluginMode bool +var ( + // ErrorTimeout returned if waiting for a condition times out + ErrorTimeout = errors.New("timeout waiting for condition") +) func SetPluginMode(value bool) { pluginMode = value @@ -250,7 +255,7 @@ func WaitForCondition(eval func() bool, interval time.Duration, timeout time.Dur } if time.Now().After(deadline) { - return fmt.Errorf("timeout waiting for condition") + return ErrorTimeout } time.Sleep(interval) diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go index ed84fb42d..bba8de5d9 100644 --- a/pkg/common/utils/utils_test.go +++ b/pkg/common/utils/utils_test.go @@ -22,6 +22,7 @@ import ( "bytes" "compress/gzip" "fmt" + "reflect" "strings" "testing" "time" @@ -49,6 +50,21 @@ func TestConvert2Pod(t *testing.T) { assert.Assert(t, pod != nil) } +func TestConvert2ConfigMap(t *testing.T) { + configMap := &v1.ConfigMap{} + result := Convert2ConfigMap(configMap) + assert.Equal(t, result != nil, true) + assert.Equal(t, reflect.DeepEqual(result, configMap), true) + + obj := struct{}{} + result = Convert2ConfigMap(obj) + assert.Equal(t, result == nil, true) + + pod := &v1.Pod{} + result = Convert2ConfigMap(pod) + assert.Equal(t, result == nil, true) +} + func TestIsAssignedPod(t *testing.T) { assigned := IsAssignedPod(&v1.Pod{ Spec: v1.PodSpec{ @@ -212,6 +228,15 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAnnotations(t *testing.T) { }, }, }, nil}, + {&v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Annotations: map[string]string{ + constants.DomainYuniKorn + "namespace.quota": "expecting JSON object", + }, + }, + }, nil}, } for _, tc := range testCases { t.Run(fmt.Sprintf("namespace: %v", tc.namespace), func(t *testing.T) { @@ -624,9 +649,11 @@ func TestGetApplicationIDFromPod(t *testing.T) { t.Run(tc.name, func(t *testing.T) { conf.GetSchedulerConf().GenerateUniqueAppIds = tc.generateUniqueAppIds SetPluginMode(false) + assert.Equal(t, IsPluginMode(), false) appID := GetApplicationIDFromPod(tc.pod) assert.Equal(t, appID, tc.expectedAppID, "Wrong appID (standard mode)") SetPluginMode(true) + assert.Equal(t, IsPluginMode(), true) appID2 := GetApplicationIDFromPod(tc.pod) assert.Equal(t, appID2, tc.expectedAppIDPluginMode, "Wrong appID (plugin mode)") }) @@ -946,6 +973,43 @@ func TestPodAlreadyBound(t *testing.T) { } } +func TestIsPodRunning(t *testing.T) { + pod := &v1.Pod{ + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + } + assert.Equal(t, IsPodRunning(pod), true) + + pod = &v1.Pod{ + Status: v1.PodStatus{ + Phase: v1.PodFailed, + }, + } + assert.Equal(t, IsPodRunning(pod), false) + + pod = &v1.Pod{ + Status: v1.PodStatus{ + Phase: v1.PodPending, + }, + } + assert.Equal(t, IsPodRunning(pod), false) + + pod = &v1.Pod{ + Status: v1.PodStatus{ + Phase: v1.PodSucceeded, + }, + } + assert.Equal(t, IsPodRunning(pod), false) + + pod = &v1.Pod{ + Status: v1.PodStatus{ + Phase: v1.PodUnknown, + }, + } + assert.Equal(t, IsPodRunning(pod), false) +} + func TestGetTaskGroupFromPodSpec(t *testing.T) { pod := &v1.Pod{ TypeMeta: metav1.TypeMeta{ @@ -1075,6 +1139,13 @@ func TestGetCoreSchedulerConfigFromConfigMap(t *testing.T) { assert.Equal(t, "test", GetCoreSchedulerConfigFromConfigMap(cm)) } +func TestGetCoreSchedulerConfigNotMapping(t *testing.T) { + cm := map[string]string{ + "unknow.yaml": "test", + } + assert.Equal(t, "", GetCoreSchedulerConfigFromConfigMap(cm)) +} + func TestGzipCompressedConfigMap(t *testing.T) { var b bytes.Buffer gzWriter := gzip.NewWriter(&b) @@ -1140,3 +1211,30 @@ func TestConvert2PriorityClass(t *testing.T) { assert.Assert(t, result != nil) assert.Equal(t, result.PreemptionPolicy, &preemptLower) } + +func TestWaitForCondition(t *testing.T) { + target := false + eval := func() bool { + return target + } + tests := []struct { + input bool + interval time.Duration + timeout time.Duration + output error + }{ + {true, time.Duration(1) * time.Second, time.Duration(2) * time.Second, nil}, + {false, time.Duration(1) * time.Second, time.Duration(2) * time.Second, ErrorTimeout}, + {true, time.Duration(3) * time.Second, time.Duration(2) * time.Second, nil}, + {false, time.Duration(3) * time.Second, time.Duration(2) * time.Second, ErrorTimeout}, + } + for _, test := range tests { + target = test.input + get := WaitForCondition(eval, test.timeout, test.interval) + if test.output == nil { + assert.NilError(t, get) + } else { + assert.Equal(t, get.Error(), test.output.Error()) + } + } +} From 8c8a4a21b9fd0805e12188c9ae4de8a3eb827d34 Mon Sep 17 00:00:00 2001 From: SP12893678 <36910625+SP12893678@users.noreply.github.com> Date: Fri, 19 Jul 2024 21:10:20 +0800 Subject: [PATCH 2/3] [YUNIKORN-2762] code review --- pkg/common/utils/utils_test.go | 43 ++++++++-------------------------- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go index bba8de5d9..cbb7a107f 100644 --- a/pkg/common/utils/utils_test.go +++ b/pkg/common/utils/utils_test.go @@ -987,27 +987,6 @@ func TestIsPodRunning(t *testing.T) { }, } assert.Equal(t, IsPodRunning(pod), false) - - pod = &v1.Pod{ - Status: v1.PodStatus{ - Phase: v1.PodPending, - }, - } - assert.Equal(t, IsPodRunning(pod), false) - - pod = &v1.Pod{ - Status: v1.PodStatus{ - Phase: v1.PodSucceeded, - }, - } - assert.Equal(t, IsPodRunning(pod), false) - - pod = &v1.Pod{ - Status: v1.PodStatus{ - Phase: v1.PodUnknown, - }, - } - assert.Equal(t, IsPodRunning(pod), false) } func TestGetTaskGroupFromPodSpec(t *testing.T) { @@ -1123,27 +1102,25 @@ func TestGetPlaceholderFlagFromPodSpec(t *testing.T) { } } -func TestGetCoreSchedulerConfigFromConfigMapNil(t *testing.T) { - assert.Equal(t, "", GetCoreSchedulerConfigFromConfigMap(nil)) -} - -func TestGetCoreSchedulerConfigFromConfigMapEmpty(t *testing.T) { - cm := map[string]string{} - assert.Equal(t, "", GetCoreSchedulerConfigFromConfigMap(cm)) -} - func TestGetCoreSchedulerConfigFromConfigMap(t *testing.T) { + // case: mapping cm := map[string]string{ "queues.yaml": "test", } assert.Equal(t, "test", GetCoreSchedulerConfigFromConfigMap(cm)) -} -func TestGetCoreSchedulerConfigNotMapping(t *testing.T) { - cm := map[string]string{ + // case: not mapping + cm = map[string]string{ "unknow.yaml": "test", } assert.Equal(t, "", GetCoreSchedulerConfigFromConfigMap(cm)) + + // case: nil + assert.Equal(t, "", GetCoreSchedulerConfigFromConfigMap(nil)) + + // case: empty + cm = map[string]string{} + assert.Equal(t, "", GetCoreSchedulerConfigFromConfigMap(cm)) } func TestGzipCompressedConfigMap(t *testing.T) { From f47a831a1ba2e92b3a91add7ce38f3a53827e5fe Mon Sep 17 00:00:00 2001 From: SP12893678 <36910625+SP12893678@users.noreply.github.com> Date: Sun, 21 Jul 2024 20:51:20 +0800 Subject: [PATCH 3/3] [YUNIKORN-2762] fix lint --- pkg/common/utils/utils_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go index cbb7a107f..adb072142 100644 --- a/pkg/common/utils/utils_test.go +++ b/pkg/common/utils/utils_test.go @@ -535,6 +535,7 @@ func TestPodUnderCondition(t *testing.T) { assert.Equal(t, PodUnderCondition(pod, condition), false) } +// nolint: funlen func TestGetApplicationIDFromPod(t *testing.T) { defer SetPluginMode(false) defer func() { conf.GetSchedulerConf().GenerateUniqueAppIds = false }()