cnauroth commented on code in PR #7567:
URL: https://github.com/apache/hadoop/pull/7567#discussion_r2059255067


##########
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestExternalCall.java:
##########
@@ -65,8 +66,12 @@ private static Configuration getConf() {
   @BeforeEach
   public void setup() {
 
-    securityManager = System.getSecurityManager();
-    System.setSecurityManager(new NoExitSecurityManager());
+    try {
+      securityManager = System.getSecurityManager();
+      System.setSecurityManager(new NoExitSecurityManager());
+    } catch (UnsupportedOperationException e) {
+      assumeTrue(false, "Test is skipped because SecurityManager cannot be set 
(JEP 411)");

Review Comment:
   Thank you for fixing this, @stoty . It seems like this will result in loss 
of test coverage on JDK 24. A couple of potential ideas:
   
   1. I think only `testCleanupTestViaToolRunner` actually depends on modifying 
security manager to catch a `System.exit` call. We could push the `assumeTrue` 
and all of the security manager modification down into just that method, and 
the other test methods could continue running on JDK 24.
   2. We could potentially modify `DistCp` so that instead of directly calling 
`System.exit`, it holds a `Consumer<Integer> exitHandler`. By default, this is 
set to `System::exit`, but we also have a `VisibleForTesting` constructor that 
accepts an override. Tests can override it to assert on the exit code instead 
of a real `System.exit`. This technique would totally remove the security 
manager dependency so that all tests can run across all JDK versions.
   
   Let me know your thoughts. Thanks!



-- 
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: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to