[YUNIKORN-2883] Improve sorters & queue_tracker funtion's test coverage

apache · Sep 20, 2024 · fb2acf3 · fb2acf3
1 parent 7337944
commit fb2acf3
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 9 deletions.
diff --git a/pkg/scheduler/objects/sorters_test.go b/pkg/scheduler/objects/sorters_test.go
@@ -236,15 +236,15 @@ func TestSortAppsFifo(t *testing.T) {
 
 	input["app-1"].askMaxPriority = 3
 	input["app-3"].askMaxPriority = 5
-	input["app-2"].SubmissionTime = input["app-3"].SubmissionTime
+	input["app-2"].SubmissionTime = input["app-3"].SubmissionTime.Add(time.Nanosecond * -5)
 	input["app-1"].SubmissionTime = input["app-3"].SubmissionTime
 	list = sortApplications(input, policies.FifoSortPolicy, false, nil)
 	/*
 	* apps order: 0, 3, 1, 2
-	* the resultType of app index is [0, 2, 3, 1]
-	* app0 with index 0, app1 with index 2, app2 with index 3 and app3 with index 1
+	* the resultType of app index is [0, 3, 1, 2]
+	* app0 with index 0, app1 with index 3, app2 with index 1 and app3 with index 2
 	 */
-	assertAppList(t, list, []int{0, 2, 3, 1}, "fifo first, priority second")
+	assertAppList(t, list, []int{0, 3, 1, 2}, "fifo first, priority second")
 }
 
 func TestSortAppsPriorityFifo(t *testing.T) {

diff --git a/pkg/scheduler/ugm/queue_tracker.go b/pkg/scheduler/ugm/queue_tracker.go
@@ -410,11 +410,7 @@ func (qt *QueueTracker) canRunApp(hierarchy []string, applicationID string, trac
 func (qt *QueueTracker) canBeRemoved() bool {
 	for _, childQT := range qt.childQueueTrackers {
 		// quick check to avoid further traversal
-		if childQT.canBeRemovedInternal() {
-			if !childQT.canBeRemoved() {
-				return false
-			}
-		} else {
+		if !childQT.canBeRemovedInternal() {
 			return false
 		}
 	}

diff --git a/pkg/scheduler/ugm/queue_tracker_test.go b/pkg/scheduler/ugm/queue_tracker_test.go
@@ -340,6 +340,26 @@ func TestCanBeRemoved(t *testing.T) {
 	removeQT := root.decreaseTrackedResource(strings.Split(queuePath1, configs.DOT), TestApp1, usage1, true)
 	assert.Assert(t, removeQT)
 	assert.Assert(t, root.canBeRemoved())
+
+	// case: child can not be removed
+	usage2, err := resources.NewResourceFromConf(map[string]string{"mem": "0", "vcore": "0"})
+	assert.NilError(t, err)
+
+	root.increaseTrackedResource(strings.Split(queuePath1, configs.DOT), TestApp1, user, usage2)
+	parentQ = root.childQueueTrackers["parent"]
+	childQ = parentQ.childQueueTrackers["child1"]
+	assert.Assert(t, !root.canBeRemoved())
+	assert.Assert(t, !parentQ.canBeRemoved())
+	assert.Assert(t, !childQ.canBeRemoved())
+
+	removeQT = root.decreaseTrackedResource(strings.Split(path5, configs.DOT), TestApp1, usage2, true)
+	parentQ = root.childQueueTrackers["parent"]
+	childQ = parentQ.childQueueTrackers["child1"]
+
+	assert.Assert(t, !removeQT)
+	assert.Assert(t, !root.canBeRemoved())
+	assert.Assert(t, !parentQ.canBeRemoved())
+	assert.Assert(t, !childQ.canBeRemoved())
 }
 
 func TestGetResourceUsageDAOInfo(t *testing.T) {
@@ -451,3 +471,9 @@ func getQTResource(qt *QueueTracker) map[string]*resources.Resource {
 	usage := qt.getResourceUsageDAOInfo("")
 	return internalGetResource(usage, resources)
 }
+
+func TestString(t *testing.T) {
+	assert.Equal(t, none.String(), "none")
+	assert.Equal(t, user.String(), "user")
+	assert.Equal(t, group.String(), "group")
+}