wilfred-s commented on code in PR #405:
URL: https://github.com/apache/yunikorn-core/pull/405#discussion_r850106211
##########
pkg/scheduler/objects/node_collection_test.go:
##########
@@ -120,3 +121,56 @@ func TestNodeCollection_GetNodes(t *testing.T) {
nodes = nc.GetNodes()
assert.Equal(t, 1, len(nodes), "list is missing node")
}
+
+func TestGetNodeSortingPolicy(t *testing.T) {
Review Comment:
This is really testing the change of the node sorting policy:
* start nsp == nil, change to fair
* changed to fair in test 1, now change to bin
What happens if I call `GetNodeSortingPolicy()` before I call
`SetNodeSortingPolicy()` ? We should have that covered in the test and make
sure we do not panic.
A better test setup would be:
```
tests = []struct {
name string
before string
after string
}
```
an empty string would be a nil or unset policy in the before case
##########
pkg/scheduler/objects/node_collection_test.go:
##########
@@ -120,3 +121,56 @@ func TestNodeCollection_GetNodes(t *testing.T) {
nodes = nc.GetNodes()
assert.Equal(t, 1, len(nodes), "list is missing node")
}
+
+func TestGetNodeSortingPolicy(t *testing.T) {
+ nc := NewNodeCollection("test")
+ weights := map[string]float64{
+ "vcore": 2.0,
+ "memory": 3.0,
+ }
+
+ var tests = []struct {
+ input string
+ except string
+ }{
+ {"fair", "fair"},
+ {"bin", "bin"},
+ }
+
+ for _, tt := range tests {
+ testname := fmt.Sprintf("Get method of base_collection:%s %s",
tt.input, tt.except)
+ t.Run(testname, func(t *testing.T) {
+ policy := NewNodeSortingPolicy(tt.input, weights)
+ nc.SetNodeSortingPolicy(policy)
+ ans := nc.GetNodeSortingPolicy()
+
+ if policy.PolicyType() != ans.PolicyType() {
+ t.Errorf("Got %d, want %d",
policy.PolicyType(), ans.PolicyType())
+ }
+ })
+ }
+}
+
+func TestGetNodeIterator(t *testing.T) {
+ // Basic node, allocation and application
+ node := newNode(nodeID1, map[string]resources.Quantity{"first": 10})
+ res :=
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5})
+ ask := newAllocationAsk("alloc-01", "app-01", res)
+ app := newApplication("app-01", "default", "root.test")
+
+ bc := initBaseCollection()
+ var nc NodeCollection = bc
+ // Register callback listener
+ node.AddListener(bc)
+ if err := nc.AddNode(node); err != nil {
+ t.Errorf("Adding a node should be worked.")
+ }
+ // Callback trigger
+ if err := node.Reserve(app, ask); err != nil {
Review Comment:
Should check that the iterator is not nil before reserving the one node in
the list.
##########
pkg/scheduler/objects/node_collection_test.go:
##########
@@ -120,3 +121,56 @@ func TestNodeCollection_GetNodes(t *testing.T) {
nodes = nc.GetNodes()
assert.Equal(t, 1, len(nodes), "list is missing node")
}
+
+func TestGetNodeSortingPolicy(t *testing.T) {
+ nc := NewNodeCollection("test")
+ weights := map[string]float64{
+ "vcore": 2.0,
+ "memory": 3.0,
+ }
+
+ var tests = []struct {
+ input string
+ except string
+ }{
+ {"fair", "fair"},
+ {"bin", "bin"},
+ }
+
+ for _, tt := range tests {
+ testname := fmt.Sprintf("Get method of base_collection:%s %s",
tt.input, tt.except)
+ t.Run(testname, func(t *testing.T) {
+ policy := NewNodeSortingPolicy(tt.input, weights)
+ nc.SetNodeSortingPolicy(policy)
Review Comment:
Changing the policy changes the node ordering. Should this not be part of
the test?
* nil sorting returns node in ??random?? order (not sure)
* fair sorting will return least loaded node first
* bin sorting should return most loaded node first.
We should have thus at least two nodes and make sure that the order they
come back in is correct after the update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
