-
Notifications
You must be signed in to change notification settings - Fork 38.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
complete pkg/scheduler/util unit test #82368
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,12 +17,16 @@ limitations under the License. | |
package util | ||
|
||
import ( | ||
"fmt" | ||
"reflect" | ||
"testing" | ||
"time" | ||
|
||
"k8s.io/api/core/v1" | ||
v1 "k8s.io/api/core/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/util/diff" | ||
"k8s.io/kubernetes/pkg/api/v1/pod" | ||
"k8s.io/kubernetes/pkg/scheduler/api" | ||
) | ||
|
||
// TestSortableList tests SortableList by storing pods in the list and sorting | ||
|
@@ -171,3 +175,92 @@ func TestGetContainerPorts(t *testing.T) { | |
} | ||
} | ||
} | ||
|
||
func TestGetPodFullName(t *testing.T) { | ||
pod := &v1.Pod{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Namespace: "test", | ||
Name: "pod", | ||
}, | ||
} | ||
got := GetPodFullName(pod) | ||
expected := fmt.Sprintf("%s_%s", pod.Name, pod.Namespace) | ||
if got != expected { | ||
t.Errorf("Got wrong full name, got: %s, expected: %s", got, expected) | ||
} | ||
} | ||
|
||
func newPriorityPodWithStartTime(name string, priority int32, startTime time.Time) *v1.Pod { | ||
return &v1.Pod{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: name, | ||
}, | ||
Spec: v1.PodSpec{ | ||
Priority: &priority, | ||
}, | ||
Status: v1.PodStatus{ | ||
StartTime: &metav1.Time{Time: startTime}, | ||
}, | ||
} | ||
} | ||
|
||
func TestGetEarliestPodStartTime(t *testing.T) { | ||
currentTime := time.Now() | ||
pod1 := newPriorityPodWithStartTime("pod1", 1, currentTime.Add(time.Second)) | ||
pod2 := newPriorityPodWithStartTime("pod2", 2, currentTime.Add(time.Second)) | ||
pod3 := newPriorityPodWithStartTime("pod3", 2, currentTime) | ||
victims := &api.Victims{ | ||
Pods: []*v1.Pod{pod1, pod2, pod3}, | ||
} | ||
startTime := GetEarliestPodStartTime(victims) | ||
if !startTime.Equal(pod3.Status.StartTime) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's better to check pod name instead of start time; and we should add some other case, e.g. all priority are the same. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
we can not get pod's name through
done |
||
t.Errorf("Got wrong earliest pod start time") | ||
} | ||
|
||
pod1 = newPriorityPodWithStartTime("pod1", 2, currentTime) | ||
pod2 = newPriorityPodWithStartTime("pod2", 2, currentTime.Add(time.Second)) | ||
pod3 = newPriorityPodWithStartTime("pod3", 2, currentTime.Add(2*time.Second)) | ||
victims = &api.Victims{ | ||
Pods: []*v1.Pod{pod1, pod2, pod3}, | ||
} | ||
startTime = GetEarliestPodStartTime(victims) | ||
if !startTime.Equal(pod1.Status.StartTime) { | ||
t.Errorf("Got wrong earliest pod start time, got %v, expected %v", startTime, pod1.Status.StartTime) | ||
} | ||
} | ||
|
||
func TestMoreImportantPod(t *testing.T) { | ||
currentTime := time.Now() | ||
pod1 := newPriorityPodWithStartTime("pod1", 1, currentTime) | ||
pod2 := newPriorityPodWithStartTime("pod2", 2, currentTime.Add(time.Second)) | ||
pod3 := newPriorityPodWithStartTime("pod3", 2, currentTime) | ||
|
||
tests := map[string]struct { | ||
p1 *v1.Pod | ||
p2 *v1.Pod | ||
expected bool | ||
}{ | ||
"Pod with higher priority": { | ||
p1: pod1, | ||
p2: pod2, | ||
expected: false, | ||
}, | ||
"Pod with older created time": { | ||
p1: pod2, | ||
p2: pod3, | ||
expected: false, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a case for pod1 vs. pod3 and expected is true. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added~ |
||
"Pods with same start time": { | ||
p1: pod3, | ||
p2: pod1, | ||
expected: true, | ||
}, | ||
} | ||
|
||
for k, v := range tests { | ||
got := MoreImportantPod(v.p1, v.p2) | ||
if got != v.expected { | ||
t.Errorf("%s failed, expected %t but got %t", k, v.expected, got) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Table driven style may be clearer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There're only two different cases, so I don't use a table to contain them. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
+1
The two cases have the same pattern which could be refactored into a table-driven style. |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/v1/corev1/?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v1
is automatically added by vscode, and there manyv1 "k8s.io/api/core/v1"
in the project, so I keep it.