dmvk commented on code in PR #24680: URL: https://github.com/apache/flink/pull/24680#discussion_r1571177099
########## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/CreatingExecutionGraphTest.java: ########## @@ -166,18 +170,19 @@ static class MockCreatingExecutionGraphContext extends MockStateWithoutExecution new StateValidator<>("Executing"); private Function< - CreatingExecutionGraph.ExecutionGraphWithVertexParallelism, - CreatingExecutionGraph.AssignmentResult> + CreatingExecutionGraph.ExecutionGraphWithVertexParallelism, + CreatingExecutionGraph.AssignmentResult> tryToAssignSlotsFunction = - e -> CreatingExecutionGraph.AssignmentResult.success(e.getExecutionGraph()); + e -> CreatingExecutionGraph.AssignmentResult.success(e.getExecutionGraph()); private GlobalFailureHandler globalFailureHandler = t -> { // No-op. }; public void setExpectWaitingForResources() { - waitingForResourcesStateValidator.expectInput((none) -> {}); + waitingForResourcesStateValidator.expectInput((none) -> { Review Comment: uuh, what's happening here 🤔 did we update formatter? ########## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveSchedulerTest.java: ########## @@ -1783,56 +1778,6 @@ void testStopWithSavepointFailsInIllegalState() throws Exception { .withCauseInstanceOf(CheckpointException.class); } - @Test - void testSavepointFailsWhenBlockingEdgeExists() throws Exception { Review Comment: do you know why / when this was introduced? it indeed doesn't seem to be related AS; is this tested somewhere else? ########## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/CreatingExecutionGraphTest.java: ########## @@ -93,8 +93,12 @@ void testNotPossibleSlotAssignmentTransitionsToWaitingForResources() { ignored -> CreatingExecutionGraph.AssignmentResult.notPossible()); context.setExpectWaitingForResources(); + final StateTrackingMockExecutionGraph executionGraph = new StateTrackingMockExecutionGraph(); + executionGraphWithVertexParallelismFuture.complete( - getGraph(new StateTrackingMockExecutionGraph())); + getGraph(executionGraph)); + + assertThat(executionGraph.getState()).isEqualTo(JobStatus.INITIALIZING); Review Comment: iiuc, the graph would be running before the change; makes sense -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org