Skip to content

Commit ee36a02

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #545 from ChristianZaccaria/utils-unit-test
Add unit tests for queuejob util functions
2 parents ab92337 + c592c59 commit ee36a02

File tree

5 files changed

+249
-11
lines changed

5 files changed

+249
-11
lines changed

‎pkg/controller/queuejob/heap.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,24 @@ import (
1919
"container/heap"
2020
"fmt"
2121
"k8s.io/client-go/tools/cache"
22+
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
2223
)
2324

2425
// Below is the implementation of the a heap. The logic is pretty much the same
2526
// as cache.heap, however, this heap does not perform synchronization. It leaves
2627
// synchronization to the SchedulingQueue.
2728

28-
type LessFunc func(interface{}, interface{}) bool
29+
type LessFunc func(qj1, qj2 *arbv1.AppWrapper) bool
2930
type KeyFunc func(obj interface{}) (string, error)
3031

3132
type heapItem struct {
32-
obj interface{} // The object which is stored in the heap.
33+
obj *arbv1.AppWrapper // The object which is stored in the heap.
3334
index int // The index of the object's key in the Heap.queue.
3435
}
3536

3637
type itemKeyValue struct {
3738
key string
38-
obj interface{}
39+
obj *arbv1.AppWrapper
3940
}
4041

4142
// heapData is an internal struct that implements the standard heap interface
@@ -121,7 +122,7 @@ type Heap struct {
121122

122123
// Add inserts an item, and puts it in the queue. The item is updated if it
123124
// already exists.
124-
func (h *Heap) Add(obj interface{}) error {
125+
func (h *Heap) Add(obj *arbv1.AppWrapper) error {
125126
key, err := h.data.keyFunc(obj)
126127
if err != nil {
127128
return cache.KeyError{Obj: obj, Err: err}
@@ -136,7 +137,7 @@ func (h *Heap) Add(obj interface{}) error {
136137
}
137138

138139
// BulkAdd adds all the items in the list to the queue.
139-
func (h *Heap) BulkAdd(list []interface{}) error {
140+
func (h *Heap) BulkAdd(list []*arbv1.AppWrapper) error {
140141
for _, obj := range list {
141142
key, err := h.data.keyFunc(obj)
142143
if err != nil {
@@ -154,7 +155,7 @@ func (h *Heap) BulkAdd(list []interface{}) error {
154155

155156
// AddIfNotPresent inserts an item, and puts it in the queue. If an item with
156157
// the key is present in the map, no changes is made to the item.
157-
func (h *Heap) AddIfNotPresent(obj interface{}) error {
158+
func (h *Heap) AddIfNotPresent(obj *arbv1.AppWrapper) error {
158159
key, err := h.data.keyFunc(obj)
159160
if err != nil {
160161
return cache.KeyError{Obj: obj, Err: err}
@@ -167,7 +168,7 @@ func (h *Heap) AddIfNotPresent(obj interface{}) error {
167168

168169
// Update is the same as Add in this implementation. When the item does not
169170
// exist, it is added.
170-
func (h *Heap) Update(obj interface{}) error {
171+
func (h *Heap) Update(obj *arbv1.AppWrapper) error {
171172
return h.Add(obj)
172173
}
173174

‎pkg/controller/queuejob/queuejob_controller_ex.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ func (qjm *XController) ScheduleNext() {
981981
pq := qjm.qjqueue.(*PriorityQueue)
982982
if qjm.qjqueue.Length() > 0 {
983983
for key, element := range pq.activeQ.data.items {
984-
qjtemp := element.obj.(*arbv1.AppWrapper)
984+
qjtemp := element.obj
985985
klog.V(4).Infof("[ScheduleNext] AfterCalc: qjqLength=%d Key=%s index=%d Priority=%.1f SystemPriority=%.1f QueueJobState=%s",
986986
qjm.qjqueue.Length(), key, element.index, float64(qjtemp.Spec.Priority), qjtemp.Status.SystemPriority, qjtemp.Status.QueueJobState)
987987
}

‎pkg/controller/queuejob/scheduling_queue.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545
"reflect"
4646
"sync"
4747

48+
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
4849
qjobv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
4950
"k8s.io/client-go/tools/cache"
5051
"k8s.io/klog/v2"
@@ -320,7 +321,7 @@ func (p *PriorityQueue) Delete(qj *qjobv1.AppWrapper) error {
320321
func (p *PriorityQueue) MoveAllToActiveQueue() {
321322
p.lock.Lock()
322323
defer p.lock.Unlock()
323-
var unschedulableQJs []interface{}
324+
var unschedulableQJs []*arbv1.AppWrapper
324325
for _, qj := range p.unschedulableQ.pods {
325326
unschedulableQJs = append(unschedulableQJs, qj)
326327
}

‎pkg/controller/queuejob/utils.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ func GetXQJFullName(qj *arbv1.AppWrapper) string {
2929
return qj.Name + "_" + qj.Namespace
3030
}
3131

32-
func HigherSystemPriorityQJ(qj1, qj2 interface{}) bool {
33-
return qj1.(*arbv1.AppWrapper).Status.SystemPriority > qj2.(*arbv1.AppWrapper).Status.SystemPriority
32+
func HigherSystemPriorityQJ(qj1, qj2 *arbv1.AppWrapper) bool {
33+
return qj1.Status.SystemPriority > qj2.Status.SystemPriority
3434
}
3535

3636
// GenerateAppWrapperCondition returns condition of a AppWrapper condition.

‎pkg/controller/queuejob/utils_test.go‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
package queuejob
2+
3+
import (
4+
"testing"
5+
6+
"github.com/onsi/gomega"
7+
8+
corev1 "k8s.io/api/core/v1"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
11+
)
12+
13+
14+
func TestGetXQJFullName(t *testing.T) {
15+
g := gomega.NewGomegaWithT(t)
16+
17+
tests := []struct {
18+
name string
19+
qj *arbv1.AppWrapper
20+
expected string
21+
}{
22+
{
23+
name: "valid qj",
24+
qj: &arbv1.AppWrapper{
25+
ObjectMeta: metav1.ObjectMeta{
26+
Name: "qjName",
27+
Namespace: "qjNamespace",
28+
},
29+
},
30+
expected: "qjName_qjNamespace",
31+
},
32+
}
33+
34+
for _, tt := range tests {
35+
t.Run(tt.name, func(t *testing.T) {
36+
result := GetXQJFullName(tt.qj)
37+
g.Expect(result).To(gomega.Equal(tt.expected))
38+
})
39+
}
40+
}
41+
42+
43+
func TestHigherSystemPriorityQJ(t *testing.T) {
44+
g := gomega.NewGomegaWithT(t)
45+
46+
tests := []struct {
47+
name string
48+
qj1 *arbv1.AppWrapper
49+
qj2 *arbv1.AppWrapper
50+
expected bool
51+
}{
52+
{
53+
name: "lower priority qj1",
54+
qj1: &arbv1.AppWrapper{
55+
Status: arbv1.AppWrapperStatus{
56+
SystemPriority: 1,
57+
},
58+
},
59+
qj2: &arbv1.AppWrapper{
60+
Status: arbv1.AppWrapperStatus{
61+
SystemPriority: 2,
62+
},
63+
},
64+
expected: false,
65+
},
66+
{
67+
name: "higher priority qj1",
68+
qj1: &arbv1.AppWrapper{
69+
Status: arbv1.AppWrapperStatus{
70+
SystemPriority: 2,
71+
},
72+
},
73+
qj2: &arbv1.AppWrapper{
74+
Status: arbv1.AppWrapperStatus{
75+
SystemPriority: 1,
76+
},
77+
},
78+
expected: true,
79+
},
80+
}
81+
82+
for _, tt := range tests {
83+
t.Run(tt.name, func(t *testing.T) {
84+
result := HigherSystemPriorityQJ(tt.qj1, tt.qj2)
85+
g.Expect(result).To(gomega.Equal(tt.expected))
86+
})
87+
}
88+
}
89+
90+
func TestGenerateAppWrapperCondition(t *testing.T) {
91+
g := gomega.NewGomegaWithT(t)
92+
93+
tests := []struct {
94+
name string
95+
conditionType arbv1.AppWrapperConditionType
96+
status corev1.ConditionStatus
97+
reason string
98+
message string
99+
expected arbv1.AppWrapperCondition
100+
}{
101+
{
102+
name: "generate condition",
103+
conditionType: arbv1.AppWrapperConditionType("Queuing"),
104+
status: corev1.ConditionTrue,
105+
reason: "reason",
106+
message: "message",
107+
expected: arbv1.AppWrapperCondition{
108+
Type: arbv1.AppWrapperConditionType("Queuing"),
109+
Status: corev1.ConditionTrue,
110+
Reason: "reason",
111+
Message: "message",
112+
},
113+
},
114+
}
115+
116+
for _, tt := range tests {
117+
t.Run(tt.name, func(t *testing.T) {
118+
result := GenerateAppWrapperCondition(tt.conditionType, tt.status, tt.reason, tt.message)
119+
120+
g.Expect(result.Type).To(gomega.Equal(tt.expected.Type))
121+
g.Expect(result.Status).To(gomega.Equal(tt.expected.Status))
122+
g.Expect(result.Reason).To(gomega.Equal(tt.expected.Reason))
123+
g.Expect(result.Message).To(gomega.Equal(tt.expected.Message))
124+
})
125+
}
126+
}
127+
128+
129+
func TestIsLastConditionDuplicate(t *testing.T) {
130+
g := gomega.NewGomegaWithT(t)
131+
132+
aw := &arbv1.AppWrapper{
133+
Status: arbv1.AppWrapperStatus{
134+
Conditions: []arbv1.AppWrapperCondition{
135+
{
136+
Type: arbv1.AppWrapperConditionType("Queuing"),
137+
Status: corev1.ConditionTrue,
138+
Reason: "reason",
139+
Message: "test",
140+
},
141+
},
142+
},
143+
}
144+
145+
tests := []struct {
146+
name string
147+
conditionType arbv1.AppWrapperConditionType
148+
status corev1.ConditionStatus
149+
reason string
150+
message string
151+
expected bool
152+
}{
153+
{
154+
name: "duplicate condition",
155+
conditionType: arbv1.AppWrapperConditionType("Queuing"),
156+
status: corev1.ConditionTrue,
157+
reason: "reason",
158+
message: "test",
159+
expected: true,
160+
},
161+
{
162+
name: "different condition",
163+
conditionType: arbv1.AppWrapperConditionType("Running"),
164+
status: corev1.ConditionFalse,
165+
reason: "noReason",
166+
message: "noMessage",
167+
expected: false,
168+
},
169+
}
170+
171+
for _, tt := range tests {
172+
t.Run(tt.name, func(t *testing.T) {
173+
result := isLastConditionDuplicate(aw, tt.conditionType, tt.status, tt.reason, tt.message)
174+
g.Expect(result).To(gomega.Equal(tt.expected))
175+
})
176+
}
177+
}
178+
179+
180+
func TestGetIndexOfMatchedCondition(t *testing.T) {
181+
g := gomega.NewGomegaWithT(t)
182+
183+
aw := &arbv1.AppWrapper{
184+
Status: arbv1.AppWrapperStatus{
185+
Conditions: []arbv1.AppWrapperCondition{
186+
{
187+
Type: arbv1.AppWrapperConditionType("Queuing"),
188+
Reason: "reason",
189+
},
190+
{
191+
Type: arbv1.AppWrapperConditionType("Running"),
192+
Reason: "reason1",
193+
},
194+
},
195+
},
196+
}
197+
198+
tests := []struct {
199+
name string
200+
conditionType arbv1.AppWrapperConditionType
201+
reason string
202+
expected int
203+
}{
204+
{
205+
name: "match reason and condition",
206+
conditionType: arbv1.AppWrapperConditionType("Queuing"),
207+
reason: "reason",
208+
expected: 0,
209+
},
210+
{
211+
name: "match reason and condition",
212+
conditionType: arbv1.AppWrapperConditionType("Running"),
213+
reason: "reason1",
214+
expected: 1,
215+
},
216+
{
217+
name: "match reason but not condition",
218+
conditionType: arbv1.AppWrapperConditionType("Running"),
219+
reason: "reason",
220+
expected: -1,
221+
},
222+
{
223+
name: "no match",
224+
conditionType: arbv1.AppWrapperConditionType("Queuing"),
225+
reason: "reason1",
226+
expected: -1,
227+
},
228+
}
229+
230+
for _, tt := range tests {
231+
t.Run(tt.name, func(t *testing.T) {
232+
result := getIndexOfMatchedCondition(aw, tt.conditionType, tt.reason)
233+
g.Expect(result).To(gomega.Equal(tt.expected))
234+
})
235+
}
236+
}

0 commit comments

Comments
 (0)