tillrohrmann commented on a change in pull request #13590:
URL: https://github.com/apache/flink/pull/13590#discussion_r503255908



##########
File path: 
flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ExecutionTest.java
##########
@@ -282,61 +278,6 @@ public void 
testSlotAllocationCancellationWhenExecutionCancelled() throws Except
                assertThat(canceledSlotRequests, equalTo(slotRequests));
        }
 
-       /**
-        * Tests that all preferred locations are calculated.
-        */
-       @Test
-       public void testAllPreferredLocationCalculation() throws Exception {
-               final TaskManagerLocation taskManagerLocation1 = new 
LocalTaskManagerLocation();
-               final TaskManagerLocation taskManagerLocation2 = new 
LocalTaskManagerLocation();
-               final TaskManagerLocation taskManagerLocation3 = new 
LocalTaskManagerLocation();
-
-               final CompletableFuture<TaskManagerLocation> locationFuture1 = 
CompletableFuture.completedFuture(taskManagerLocation1);
-               final CompletableFuture<TaskManagerLocation> locationFuture2 = 
new CompletableFuture<>();
-               final CompletableFuture<TaskManagerLocation> locationFuture3 = 
new CompletableFuture<>();
-
-               final Execution execution = 
getExecution(Arrays.asList(locationFuture1, locationFuture2, locationFuture3));
-
-               CompletableFuture<Collection<TaskManagerLocation>> 
preferredLocationsFuture = 
execution.calculatePreferredLocations(LocationPreferenceConstraint.ALL);
-
-               assertFalse(preferredLocationsFuture.isDone());
-
-               locationFuture3.complete(taskManagerLocation3);
-
-               assertFalse(preferredLocationsFuture.isDone());
-
-               locationFuture2.complete(taskManagerLocation2);
-
-               assertTrue(preferredLocationsFuture.isDone());
-
-               final Collection<TaskManagerLocation> preferredLocations = 
preferredLocationsFuture.get();
-
-               assertThat(preferredLocations, 
containsInAnyOrder(taskManagerLocation1, taskManagerLocation2, 
taskManagerLocation3));
-       }
-
-       /**
-        * Tests that any preferred locations are calculated.
-        */
-       @Test
-       public void testAnyPreferredLocationCalculation() throws Exception {
-               final TaskManagerLocation taskManagerLocation1 = new 
LocalTaskManagerLocation();
-               final TaskManagerLocation taskManagerLocation3 = new 
LocalTaskManagerLocation();
-
-               final CompletableFuture<TaskManagerLocation> locationFuture1 = 
CompletableFuture.completedFuture(taskManagerLocation1);
-               final CompletableFuture<TaskManagerLocation> locationFuture2 = 
new CompletableFuture<>();
-               final CompletableFuture<TaskManagerLocation> locationFuture3 = 
CompletableFuture.completedFuture(taskManagerLocation3);
-
-               final Execution execution = 
getExecution(Arrays.asList(locationFuture1, locationFuture2, locationFuture3));
-
-               CompletableFuture<Collection<TaskManagerLocation>> 
preferredLocationsFuture = 
execution.calculatePreferredLocations(LocationPreferenceConstraint.ANY);
-
-               assertTrue(preferredLocationsFuture.isDone());
-
-               final Collection<TaskManagerLocation> preferredLocations = 
preferredLocationsFuture.get();
-
-               assertThat(preferredLocations, 
containsInAnyOrder(taskManagerLocation1, taskManagerLocation3));
-       }
-

Review comment:
       Just to confirm: We can remove these tests because we no longer use 
`LocationPreferenceConstraint.ANY` in the new scheduler, right?

##########
File path: 
flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/scheduler/ScheduleWithCoLocationHintTest.java
##########
@@ -306,25 +306,25 @@ public void testGetsNonLocalFromSharingGroupFirst() 
throws Exception {
 
                // schedule something into the shared group so that both 
instances are in the sharing group
                LogicalSlot s1 = testingSlotProvider.allocateSlot(
-                               new ScheduledUnit(getExecution(jid1, 0, 2, 
sharingGroup, loc1), sharingGroup.getSlotSharingGroupId()), 
slotProfileForLocation(loc1), TestingUtils.infiniteTime()).get();
+                               new ScheduledUnit(getExecution(jid1, 0, 2, 
sharingGroup), sharingGroup.getSlotSharingGroupId()), 
slotProfileForLocation(loc1), TestingUtils.infiniteTime()).get();
                LogicalSlot s2 = testingSlotProvider.allocateSlot(
-                               new ScheduledUnit(getExecution(jid1, 1, 2, 
sharingGroup, loc2), sharingGroup.getSlotSharingGroupId()), 
slotProfileForLocation(loc2), TestingUtils.infiniteTime()).get();
+                               new ScheduledUnit(getExecution(jid1, 1, 2, 
sharingGroup), sharingGroup.getSlotSharingGroupId()), 
slotProfileForLocation(loc2), TestingUtils.infiniteTime()).get();
 
                // schedule one locally to instance 1
                LogicalSlot s3 = testingSlotProvider.allocateSlot(
-                               new ScheduledUnit(getExecution(jid2, 0, 2, 
sharingGroup, loc1), sharingGroup.getSlotSharingGroupId(), cc1), 
slotProfileForLocation(loc1), TestingUtils.infiniteTime()).get();
+                               new ScheduledUnit(getExecution(jid2, 0, 2, 
sharingGroup), sharingGroup.getSlotSharingGroupId(), cc1), 
slotProfileForLocation(loc1), TestingUtils.infiniteTime()).get();
 
                // schedule with co location constraint (yet unassigned) and a 
preference for
                // instance 1, but it can only get instance 2
                LogicalSlot s4 = testingSlotProvider.allocateSlot(
-                               new ScheduledUnit(getExecution(jid2, 1, 2, 
sharingGroup, loc1), sharingGroup.getSlotSharingGroupId(), cc2), 
slotProfileForLocation(loc1), TestingUtils.infiniteTime()).get();
+                               new ScheduledUnit(getExecution(jid2, 1, 2, 
sharingGroup), sharingGroup.getSlotSharingGroupId(), cc2), 
slotProfileForLocation(loc1), TestingUtils.infiniteTime()).get();
 
                // schedule something into the assigned co-location constraints 
and check that they override the
                // other preferences
                LogicalSlot s5 = testingSlotProvider.allocateSlot(
-                               new ScheduledUnit(getExecution(jid3, 0, 2, 
sharingGroup, loc2), sharingGroup.getSlotSharingGroupId(), cc1), 
slotProfileForLocation(loc2), TestingUtils.infiniteTime()).get();
+                               new ScheduledUnit(getExecution(jid3, 0, 2, 
sharingGroup), sharingGroup.getSlotSharingGroupId(), cc1), 
slotProfileForLocation(loc2), TestingUtils.infiniteTime()).get();
                LogicalSlot s6 = testingSlotProvider.allocateSlot(
-                               new ScheduledUnit(getExecution(jid3, 1, 2, 
sharingGroup, loc1), sharingGroup.getSlotSharingGroupId(), cc2), 
slotProfileForLocation(loc1), TestingUtils.infiniteTime()).get();

Review comment:
       Aren't we changing the semantics of this test by saying that we don't 
have any preferred location preferences?




----------------------------------------------------------------
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]


Reply via email to