XComp commented on code in PR #24082:
URL: https://github.com/apache/flink/pull/24082#discussion_r1453558112
##########
flink-runtime/src/test/java/org/apache/flink/runtime/minicluster/MiniClusterITCase.java:
##########
@@ -149,7 +174,15 @@ private void runHandleJobsWhenNotEnoughSlots(final
JobGraph jobGraph) throws Exc
try (final MiniCluster miniCluster = new MiniCluster(cfg)) {
miniCluster.start();
- miniCluster.executeJobBlocking(jobGraph);
+ assertThatThrownBy(() -> miniCluster.executeJobBlocking(jobGraph))
+ .isInstanceOf(JobExecutionException.class)
+ .hasMessageContaining("Job execution failed")
+ .extracting(Throwable::getCause)
+ .extracting(FlinkAssertions::chainOfCauses,
FlinkAssertions.STREAM_THROWABLE)
+ .anySatisfy(
+ cause ->
+ assertThat(cause)
+
.isInstanceOf(NoResourceAvailableException.class));
Review Comment:
You're right, it would be more explicit. But `MiniClusterITCase` is an
integration test. Here, the relevant check should be to verify that we run into
a `NoResourceAvailableException`. I consider the `TimeoutException` an
implementation detail. Especially because it's not implemented like that in the
`AdaptiveScheduler`. Having additional tests to cover that would be
out-of-scope for an integration test and should be covered in a unit test in my
opinion.
--
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]
