kingamarton commented on a change in pull request #246:
URL:
https://github.com/apache/incubator-yunikorn-k8shim/pull/246#discussion_r598683618
##########
File path: pkg/appmgmt/general/general_test.go
##########
@@ -526,3 +527,269 @@ func TestGetOwnerReferences(t *testing.T) {
assert.Equal(t, returnedOwnerRefs[0].Kind, "Pod", "Unexpected owner
reference Kind")
assert.Equal(t, returnedOwnerRefs[0].APIVersion,
v1.SchemeGroupVersion.String(), "Unexpected owner reference Kind")
}
+
+// nolint: funlen
+func TestListApplication(t *testing.T) {
Review comment:
I agree with @wilfred-s. When possible we should use table driven tests.
Is much easier to read, and it will be shorter as well, so we might not need to
skip the lint funlen check, if we do, then maybe it worth to split the test
instead of just ignoring it.
##########
File path: pkg/appmgmt/general/general.go
##########
@@ -293,15 +292,8 @@ func (os *Manager) deletePod(obj interface{}) {
}
func (os *Manager) ListApplications()
(map[string]interfaces.ApplicationMetadata, error) {
- // selector: applicationID exist
- slt := labels.NewSelector()
- req, err := labels.NewRequirement(constants.LabelApplicationID,
selection.Exists, nil)
- if err != nil {
- return nil, err
- }
- slt = slt.Add(*req)
Review comment:
Instead of removing the label selector I think we should modify the
selector to check for either appID, or spark-app-selector.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
