stoty commented on code in PR #7512:
URL: https://github.com/apache/hadoop/pull/7512#discussion_r2007235127


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java:
##########
@@ -311,6 +312,12 @@ public void testStartLocalizer() throws IOException {
           .build());
 
       List<String> result=readMockParams();
+
+      if (Shell.isJavaVersionAtLeast(17)) {
+        
assertTrue(result.remove("--add-exports=java.base/sun.net.dns=ALL-UNNAMED"));
+        
assertTrue(result.remove("--add-exports=java.base/sun.net.util=ALL-UNNAMED"));

Review Comment:
   Thank you @pan3793 .
   
   JDK17 options are defined at several locations.
   Other than the one you mentioned, they are also defined in 
org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer
 which only adds 
   these two, and apparently these tests trigger that one.
   
   If I added all three, the tests would fail, as 
--add-opens=java.base/java.lang=ALL-UNNAMED is never added for those test cases.
   
   This also bothers me, I have opened HADOOP-19505 earlier to track this. 
(Though I have no immediate plans to work on that)



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to