[GitHub] [maven-surefire] CMoH commented on a change in pull request #387: [SUREFIRE-1939] Fix NPE in SystemUtils.toJdkHomeFromJvmExec if java home has 2 components

[GitBox <gi...@apache.org>]

CMoH commented on a change in pull request #387:
URL: https://github.com/apache/maven-surefire/pull/387#discussion_r773444482



##########
File path: surefire-booter/src/test/java/org/apache/maven/surefire/booter/SystemUtilsTest.java
##########
@@ -98,6 +98,15 @@ public void incorrectJdkPath()
             assertThat( SystemUtils.isJava9AtLeast( incorrect.getAbsolutePath() ) ).isFalse();
         }
 
+        @Test
+        public void incorrectJdkPathShouldNotNPE()

Review comment:
       Well, the reason I submitted this is because unit tests fail, due to this: on my machine JAVA_HOME is `/opt/openjdk-bin-11/`. Unit test `incorrectJdkPath()` works as follows:
   
   ```
           @Test
           public void incorrectJdkPath()
           {
               File jre = new File( System.getProperty( "java.home" ) );
               File jdk = jre.getParentFile();
               File incorrect = jdk.getParentFile();
               assertThat( SystemUtils.isJava9AtLeast( incorrect.getAbsolutePath() ) ).isFalse();
           }
   ```
   
   This leads to `incorrect=/`, which reaches `toJdkHomeFromJvmExec("/")`, which calls again `getParent("/")`, which yields null, and next `getName()` is called on this null. It is clear from the implementation of `toJdkHomeFromJvmExec()` that returning null is fine when this jvmExec is not found. I believe this is the case here.
   
   Also, the test is about incorrect JDK paths, and such a path is an incorrect JDK path. However, one incorrect case crashes with a non-informative NPE. I would say a "cannot find jvm executable" error on the return-null path would be more helpful for users that misconfigured their systems.




-- 
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: issues-unsubscribe@maven.apache.org

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