You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/10/12 12:50:41 UTC
[GitHub] [flink] tillrohrmann commented on a change in pull request #13590: [FLINK-19570][tests] Avoid directly creating ExecutionJobVertex and ExecutionVertex via constructors in tests
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:
users@infra.apache.org