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

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

CMoH commented on code in PR #387:
URL: https://github.com/apache/maven-surefire/pull/387#discussion_r1061962311


##########
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()
+        {
+            File jre = new File( "/opt/jdk" );
+            File jdk = jre.getParentFile();

Review Comment:
   Well, as far as the new test goes, see https://github.com/apache/maven-surefire/pull/387#discussion_r748755474. Feel free to remove the extra test.
   
   The reason for submitting this is because I had another issue, but after the initial clone of this plugin I got a NPE at the first build. I hoped fixing this would help others. From my perspective, this null-check is both conservative and more than insignificant, in short not worthy of this elaborate debate about it.



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