wilfred-s commented on code in PR #472:
URL: https://github.com/apache/yunikorn-core/pull/472#discussion_r1045400883
##########
pkg/scheduler/objects/application_test.go:
##########
@@ -90,15 +90,15 @@ func TestNewApplication(t *testing.T) {
if app.rmEventHandler == nil {
t.Fatal("non nil handler was not set in the new app")
}
- assertUserGroupResource(t, getTestUserGroup(), nil)
+ assertUserGroupResource(t, getTestUserGroup("testuser",
[]string{"testgroup"}), nil, nil, 0)
Review Comment:
Same as before: if you need the ability to check user and group separately
you add a new function for it instead of changing 50+ callers.
##########
pkg/scheduler/objects/application_state_test.go:
##########
@@ -29,7 +29,7 @@ import (
func TestAcceptStateTransition(t *testing.T) {
// Accept only from new
- app := newApplication("app-00001", "default", "root.a")
+ app := newApplication("app-00001", "default", "root.a", "testuser",
[]string{"testgroup"})
Review Comment:
Instead of changing all calls (70+ invocations) which already use this same
user as they initialise the app indirectly via the calls:
```
newApplication(...)
newApplicationWithTags(...)
NewApplication(siApp, getTestUserGroup(), nil, "")
```
Add a new call that allows specifying the user and group.
##########
pkg/scheduler/objects/utilities_test.go:
##########
@@ -224,14 +224,14 @@ func newAllocationAskAll(allocKey, appID, taskGroup
string, res *resources.Resou
return NewAllocationAskFromSI(ask)
}
-func getTestUserGroup() security.UserGroup {
- return security.UserGroup{User: "testuser", Groups:
[]string{"testgroup"}}
+func getTestUserGroup(userName string, groupNameList []string)
security.UserGroup {
Review Comment:
Same here: use a separate function `getUserGroup(userName string,
groupNameList []string) security.UserGroup` You can even make
`getTestUserGroup()` call that one with the default test user details:
`"testuser", []string{"testgroup"}`
--
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]
