MartijnVisser commented on code in PR #29533:
URL: https://github.com/apache/flink/pull/29533#discussion_r4334493942
##########
flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/slotmanager/FineGrainedSlotManagerTest.java:
##########
@@ -935,6 +939,141 @@ private void testAccessMetricValueDuringItsUnregister(
});
}
+ /**
+ * Tests that {@code runTest} waits for {@code close()} while a
requirements check is still
Review Comment:
Dropped them. The numbers from
`testRunTestWaitsForCloseWhileRequirementsCheckIsRunning` stay in the PR body.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/slotmanager/FineGrainedSlotManagerTestBase.java:
##########
@@ -234,19 +230,22 @@ protected final void runTest(RunnableWithException
testMethod) throws Exception
resourceEventListenerBuilder.build(),
blockedTaskManagerChecker));
- testMethod.run();
-
- CompletableFuture<Void> closeFuture = new CompletableFuture<>();
- runInMainThread(
- () -> {
- try {
- slotManager.close();
- } catch (Exception e) {
- closeFuture.completeExceptionally(e);
- }
- closeFuture.complete(null);
- });
- FutureUtils.assertNoException(closeFuture);
+ Throwable failure = null;
+ try {
+ testMethod.run();
+ } catch (Throwable t) {
+ failure = t;
+ }
+ // Wait for close(), so afterAll cannot shut the executor down
under a running
+ // requirements check. A failing close() does not hide the test's
own failure.
+ try {
+ FutureUtils.runAsync(slotManager::close,
mainThreadExecutor).get();
Review Comment:
Done. The helper now takes a `RunnableWithException`, since `close()` throws.
--
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]
