wilfred-s commented on a change in pull request #372:
URL:
https://github.com/apache/incubator-yunikorn-core/pull/372#discussion_r815581130
##########
File path: pkg/webservice/handlers_test.go
##########
@@ -607,6 +607,21 @@ func TestGetClusterUtilJSON(t *testing.T) {
assert.NilError(t, err, "Error when load clusterInfo from config")
assert.Equal(t, 1, len(schedulerContext.GetPartitionMapClone()))
+ // check build information of RM
+ BuildInfoMap := make(map[string]string)
Review comment:
NIT: We do not have to export this `buildInfoMap` is enough.
##########
File path: pkg/scheduler/context_test.go
##########
@@ -255,6 +255,49 @@ func TestContext_AddNode(t *testing.T) {
}
}
+func TestContext_AddRMBuildInformation(t *testing.T) {
+ context := createTestContext(t, pName)
+
+ rmID1 := "myCluster1"
+ BuildInfoMap1 := make(map[string]string)
Review comment:
NIT: We do not have to export this `buildInfoMap` is enough.
##########
File path: pkg/scheduler/context.go
##########
@@ -164,6 +173,15 @@ func (cc *ClusterContext) processRMRegistrationEvent(event
*rmevent.RMRegistrati
cc.policyGroup = policyGroup
configs.ConfigContext.Set(policyGroup, conf)
+ // store the build information of RM
+ if cc.rmInfos == nil {
+ cc.rmInfos = make(map[string]*RMInformation)
+ }
+ cc.rmInfos[rmID] = &RMInformation{
+ RMBuildInformation: event.Registration.BuildInfo,
+ }
+ cc.rmInfos[rmID].RMBuildInformation["rmId"] = rmID
+
Review comment:
Instead of duplicating this from `SetRMInfos()` we should call it here.
##########
File path: pkg/scheduler/context_test.go
##########
@@ -255,6 +255,49 @@ func TestContext_AddNode(t *testing.T) {
}
}
+func TestContext_AddRMBuildInformation(t *testing.T) {
+ context := createTestContext(t, pName)
+
+ rmID1 := "myCluster1"
+ BuildInfoMap1 := make(map[string]string)
+ BuildInfoMap1["buildDate"] = "2006-01-02T15:04:05-0700"
+ BuildInfoMap1["buildVersion"] = "latest"
+ BuildInfoMap1["isPluginVersion"] = "false"
+ rm1 := &si.RegisterResourceManagerRequest{
+ RmID: rmID1,
+ PolicyGroup: "policygroup",
+ Version: "0.0.2",
+ BuildInfo: BuildInfoMap1,
+ }
Review comment:
Leverage the `SetRMInfos()` call that you have added. Create the test
context as you did. Call `SetRMInfos()` with the buildInfoMap created here.
The first one should give a length of 1 etc (asserts like you have)
Create a second buildInfoMap object call `SetRMInfos()` again make sure that
you have two objects in the list.
Update a value in one of the two maps (like buildVersion is 1.0.0) Call
`SetRMInfos()` again with the updated map and make sure it is replaced as
expected.
That will give us a full test of the code
--
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]