pbacsko commented on code in PR #1022:
URL: https://github.com/apache/yunikorn-core/pull/1022#discussion_r2138437831
##########
pkg/scheduler/objects/sorted_asks_test.go:
##########
@@ -98,6 +98,106 @@ func TestInsertRemove(t *testing.T) {
assert.Equal(t, 99, len(sorted))
}
+func TestRemoveWithSamePriorityAndTime(t *testing.T) {
+ // Create a sorted requests list
+ sorted := sortedRequests{}
+
+ // Create allocations with same priority and creation time
+ // but different allocation keys
+ baseTime := time.Now()
+ alloc1 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-1",
+ }
+ alloc2 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-2",
+ }
+ alloc3 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-3",
+ }
+
+ // Insert the allocations
+ sorted.insert(alloc1)
+ sorted.insert(alloc2)
+ sorted.insert(alloc3)
+
+ // Verify all allocations are in the list
+ assert.Equal(t, 3, len(sorted))
+ assert.Assert(t, askPresent(alloc1, sorted), "alloc1 should be present")
+ assert.Assert(t, askPresent(alloc2, sorted), "alloc2 should be present")
+ assert.Assert(t, askPresent(alloc3, sorted), "alloc3 should be present")
+
+ // Try to remove alloc2
+ sorted.remove(alloc2)
+
+ // Verify alloc2 is removed but alloc1 and alloc3 are still there
+ assert.Equal(t, 2, len(sorted))
+ assert.Assert(t, askPresent(alloc1, sorted), "alloc1 should still be
present")
+ assert.Assert(t, !askPresent(alloc2, sorted), "alloc2 should be
removed")
+ assert.Assert(t, askPresent(alloc3, sorted), "alloc3 should still be
present")
+
+ // Try to remove alloc1
+ sorted.remove(alloc1)
+
+ // Verify alloc1 is removed and only alloc3 remains
+ assert.Equal(t, 1, len(sorted))
+ assert.Assert(t, !askPresent(alloc1, sorted), "alloc1 should be
removed")
+ assert.Assert(t, askPresent(alloc3, sorted), "alloc3 should still be
present")
+
+ // Try to remove alloc3
+ sorted.remove(alloc3)
+
+ // Verify all allocations are removed
+ assert.Equal(t, 0, len(sorted))
+ assert.Assert(t, !askPresent(alloc3, sorted), "alloc3 should be
removed")
+}
+
+func TestRemoveWithSamePriorityAndTimeButDifferentInstances(t *testing.T) {
+ // Create a sorted requests list
+ sorted := sortedRequests{}
+
+ // Create allocations with same priority and creation time
+ baseTime := time.Now()
+
+ // Insert first set of allocations
+ alloc1 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-1",
+ }
+ alloc2 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-2",
+ }
+ sorted.insert(alloc1)
+ sorted.insert(alloc2)
+
+ // Verify both allocations are in the list
+ assert.Equal(t, 2, len(sorted))
+
+ // Create a new instance of alloc2 with the same key, priority, and time
+ // This simulates what might happen in the application when removing an
allocation
+ alloc2Copy := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-2",
+ }
+
+ // Try to remove using the copy
+ sorted.remove(alloc2Copy)
+
+ // Verify alloc2 is removed
+ assert.Equal(t, 1, len(sorted))
+ assert.Assert(t, !askPresent(alloc2, sorted), "alloc2 should be
removed")
+ assert.Assert(t, askPresent(alloc1, sorted), "alloc1 should still be
present")
Review Comment:
This can be merged to the previous test as Yu-Lin said.
##########
pkg/scheduler/objects/sorted_asks_test.go:
##########
@@ -98,6 +98,106 @@ func TestInsertRemove(t *testing.T) {
assert.Equal(t, 99, len(sorted))
}
+func TestRemoveWithSamePriorityAndTime(t *testing.T) {
+ // Create a sorted requests list
+ sorted := sortedRequests{}
+
+ // Create allocations with same priority and creation time
+ // but different allocation keys
+ baseTime := time.Now()
+ alloc1 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-1",
+ }
+ alloc2 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-2",
+ }
+ alloc3 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-3",
+ }
+
+ // Insert the allocations
+ sorted.insert(alloc1)
+ sorted.insert(alloc2)
+ sorted.insert(alloc3)
+
+ // Verify all allocations are in the list
+ assert.Equal(t, 3, len(sorted))
+ assert.Assert(t, askPresent(alloc1, sorted), "alloc1 should be present")
+ assert.Assert(t, askPresent(alloc2, sorted), "alloc2 should be present")
+ assert.Assert(t, askPresent(alloc3, sorted), "alloc3 should be present")
+
+ // Try to remove alloc2
+ sorted.remove(alloc2)
+
+ // Verify alloc2 is removed but alloc1 and alloc3 are still there
+ assert.Equal(t, 2, len(sorted))
+ assert.Assert(t, askPresent(alloc1, sorted), "alloc1 should still be
present")
+ assert.Assert(t, !askPresent(alloc2, sorted), "alloc2 should be
removed")
+ assert.Assert(t, askPresent(alloc3, sorted), "alloc3 should still be
present")
+
+ // Try to remove alloc1
+ sorted.remove(alloc1)
+
+ // Verify alloc1 is removed and only alloc3 remains
+ assert.Equal(t, 1, len(sorted))
+ assert.Assert(t, !askPresent(alloc1, sorted), "alloc1 should be
removed")
+ assert.Assert(t, askPresent(alloc3, sorted), "alloc3 should still be
present")
+
+ // Try to remove alloc3
+ sorted.remove(alloc3)
+
+ // Verify all allocations are removed
+ assert.Equal(t, 0, len(sorted))
+ assert.Assert(t, !askPresent(alloc3, sorted), "alloc3 should be
removed")
+}
+
+func TestRemoveWithSamePriorityAndTimeButDifferentInstances(t *testing.T) {
+ // Create a sorted requests list
+ sorted := sortedRequests{}
+
+ // Create allocations with same priority and creation time
+ baseTime := time.Now()
+
+ // Insert first set of allocations
+ alloc1 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-1",
+ }
+ alloc2 := &Allocation{
+ createTime: baseTime,
+ priority: 10,
+ allocationKey: "alloc-2",
+ }
+ sorted.insert(alloc1)
+ sorted.insert(alloc2)
+
+ // Verify both allocations are in the list
+ assert.Equal(t, 2, len(sorted))
+
+ // Create a new instance of alloc2 with the same key, priority, and time
+ // This simulates what might happen in the application when removing an
allocation
+ alloc2Copy := &Allocation{
Review Comment:
This can be merged to the previous test as Yu-Lin said.
--
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]
