wilfred-s commented on code in PR #701:
URL: https://github.com/apache/yunikorn-core/pull/701#discussion_r1402889139
##########
pkg/scheduler/objects/sorters_test.go:
##########
@@ -385,6 +385,18 @@ func TestSortAppsStateAware(t *testing.T) {
assert.NilError(t, err, "state change failed for app-3")
list = sortApplications(input, policies.StateAwarePolicy, false, nil)
assertAppListLength(t, list, []string{appID0, appID1, appID3}, "state
not app-2")
+
+ // move 3rd to starting: 1st, 3rd and 4th in that order
+ err = input[appID2].HandleApplicationEvent(RunApplication)
+ assert.NilError(t, err, "state change failed for app-2")
+ list = sortApplications(input, policies.StateAwarePolicy, false, nil)
+ assertAppListLength(t, list, []string{appID0, appID2, appID3}, "state
not app-1")
+
+ // move 2nd to starting: 1st, 2nd and 4th in that order
+ err = input[appID1].HandleApplicationEvent(RunApplication)
+ assert.NilError(t, err, "state change failed for app-1")
+ list = sortApplications(input, policies.StateAwarePolicy, false, nil)
+ assertAppListLength(t, list, []string{appID0, appID1, appID3}, "state
not app-2")
Review Comment:
This is an incorrect test case. You have two applications in a Starting
state. In the Stateaware setup that cannot and must not happen as it breaks the
guarantee that is given.
Both "app-1" and "app-2" have been progressed to Starting. You can only do
this by manipulating the application states outside of the normal flow.
Since you need to move from Accepted to Starting it will not happen.
--
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]
