zentol commented on code in PR #22548:
URL: https://github.com/apache/flink/pull/22548#discussion_r1190866726


##########
flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/testutils/executor/TestExecutorExtension.java:
##########
@@ -47,8 +58,26 @@ public T getExecutor() {
 
     @Override
     public void afterAll(ExtensionContext context) throws Exception {
+        gracefulShutdown(executorService, LOG);
+    }
+
+    static void gracefulShutdown(@Nullable ExecutorService executorService, 
Logger logger) {

Review Comment:
   I can see where you're coming from, but think that for most tests this 
probably isn't relevant.
   If an executor is used for the setup of some component that requires it then 
the test usually doesn't know what is even being scheduled.
   
   This should only matter in tests that really want to drill down into the 
scheduling of operations aka "if I do _this_ then exactly _that and nothing 
else_ is being scheduled", and those are rather rare I think. The 
FutureUtilsTest is an example where this could be interesting.
   An opt-in flag may be more appropriate.



-- 
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]

Reply via email to