tillrohrmann commented on a change in pull request #13571:
URL: https://github.com/apache/flink/pull/13571#discussion_r502425179
##########
File path:
flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java
##########
@@ -359,6 +360,32 @@ ContainerStatus createTestingContainerStatus(final
ContainerId containerId) {
}
}
+ @Test
+ public void testRunAsyncCausesFatalError() throws Exception {
+ new Context() {{
+ runTest(() -> {
+ // allocate a new container
+ registerSlotRequest(resourceManager,
rmServices, resourceProfile1, taskHost);
+ Container testingContainer =
createTestingContainer();
+
resourceManager.onContainersAllocated(ImmutableList.of(testingContainer));
+
+ // let client throw exception when requesting a
new container
+
testingYarnAMRMClientAsync.setAddContainerRequestConsumer((ignored1, ignored2)
-> {
+ throw new RuntimeException("Expected
Exception");
+ });
+
+ // complete the container and request a new one
+ ContainerStatus testingContainerStatus =
createTestingContainerStatus(testingContainer.getId());
+
resourceManager.onContainersCompleted(Collections.singletonList(testingContainerStatus));
+
+ Throwable t =
testingFatalErrorHandler.getErrorFuture().get(2000L, TimeUnit.MILLISECONDS);
+ assertThat(ExceptionUtils.findThrowable(t,
IllegalStateException.class).isPresent(), is(true));
Review comment:
Sure that the thrown `RuntimeException` will trigger an`
IllegalStateException`?
##########
File path:
flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java
##########
@@ -359,6 +360,32 @@ ContainerStatus createTestingContainerStatus(final
ContainerId containerId) {
}
}
+ @Test
+ public void testRunAsyncCausesFatalError() throws Exception {
+ new Context() {{
+ runTest(() -> {
+ // allocate a new container
+ registerSlotRequest(resourceManager,
rmServices, resourceProfile1, taskHost);
+ Container testingContainer =
createTestingContainer();
+
resourceManager.onContainersAllocated(ImmutableList.of(testingContainer));
+
+ // let client throw exception when requesting a
new container
+
testingYarnAMRMClientAsync.setAddContainerRequestConsumer((ignored1, ignored2)
-> {
+ throw new RuntimeException("Expected
Exception");
+ });
+
+ // complete the container and request a new one
+ ContainerStatus testingContainerStatus =
createTestingContainerStatus(testingContainer.getId());
+
resourceManager.onContainersCompleted(Collections.singletonList(testingContainerStatus));
Review comment:
I think the test passes also w/o these lines. Can we make it explicit
where the exception is thrown which is reported via the
`TestingFatalErrorHandler`?
----------------------------------------------------------------
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:
us...@infra.apache.org
