pbacsko commented on code in PR #897:
URL: https://github.com/apache/yunikorn-core/pull/897#discussion_r1690911178
##########
pkg/webservice/handlers_test.go:
##########
@@ -1690,6 +1694,28 @@ func TestGetApplicationHandler(t *testing.T) {
assert.Equal(t, http.StatusBadRequest, resp.statusCode, statusCodeError)
assert.Equal(t, errInfo.Message, "invalid URL escape \"%Zt\"",
jsonMessageError)
assert.Equal(t, errInfo.StatusCode, http.StatusBadRequest)
+
+ // test additional application details
+ var req6 *http.Request
+ req6, err = http.NewRequest("GET",
"/ws/v1/partition/default/queue/root.default/application/app-1",
strings.NewReader(""))
+ assert.NilError(t, err, "HTTP request create failed")
+ req6 = req6.WithContext(context.WithValue(req.Context(),
httprouter.ParamsKey, httprouter.Params{
+ httprouter.Param{Key: "partition", Value:
partitionNameWithoutClusterID},
+ httprouter.Param{Key: "queue", Value: "root.default"},
+ httprouter.Param{Key: "application", Value: "app-1"},
+ }))
+ assert.NilError(t, err, "Get Application Handler request failed")
+ resp6 := &MockResponseWriter{}
+ var appDao *dao.ApplicationDAOInfo
+ getApplication(resp6, req6)
+ appSummary := app.GetApplicationSummary(partitionNameWithoutClusterID)
+ err = json.Unmarshal(resp6.outputBytes, &appDao)
+ assert.NilError(t, err, unmarshalError)
+ assert.Equal(t, "app-1", appDao.ApplicationID)
+ assert.Equal(t, app.StartTime().UnixMilli(), appDao.StartTime)
+ assert.Equal(t, appSummary.ResourceUsage.String(),
appDao.ResourceUsage.String())
+ assert.Equal(t, appSummary.PreemptedResource.String(),
appDao.PreemptedResource.String())
+ assert.Equal(t, appSummary.PlaceholderResource.String(),
appDao.PlaceholderResource.String())
Review Comment:
@richscott yes it's a different type I just found out...
I'd be happier with sth like `EqualsTracked()` created for
`TrackedResource`. You can put it inside `tracked_resource.go`. The first layer
is the extra code, the rest can be taken from the existing `Equals()`.
Eg.
```
func EqualsTracked(left, right *TrackedResource) bool {
if left == right {
return true
}
if left == nil || right == nil {
return false
}
equals := true
for k, v := range left.TrackedResourceMap {
inner, ok := right.TrackedResourceMap[k]
if !ok {
return false
}
equals = equals && equalsMapContents(v, inner)
}
return equals
}
// basically the same as Equals()
func equalsMapContents(left, right map[string]int64) bool {
if left == nil || right == nil { // probably this is not needed
return false
}
for k, v := range left {
if right[k] != v {
return false
}
}
for k, v := range right {
if left[k] != v {
return false
}
}
return true
}
```
I haven't tried this, but the main point is the approach. It would be much
better if `TrackedResourceMap` was actually `map[string]*Resource` because we
could re-use `Equals()` but this can be improved in a follow-up JIRA.
Since this is a new method, we can get away with missing unit tests, those
can be added in the follow-up, too.
--
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]
