You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by sj...@apache.org on 2022/12/30 15:14:48 UTC

[maven-enforcer] branch master updated: [MENFORCER-445] Include JAVA_HOME location in the Java rule failure messages

This is an automated email from the ASF dual-hosted git repository.

sjaranowski pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-enforcer.git


The following commit(s) were added to refs/heads/master by this push:
     new b788a52  [MENFORCER-445] Include JAVA_HOME location in the Java rule failure messages
b788a52 is described below

commit b788a52f4eedbd64ec746cb12ab5fba8539c4342
Author: Petr Široký <pe...@pm.me>
AuthorDate: Fri Dec 30 08:50:32 2022 +0100

    [MENFORCER-445] Include JAVA_HOME location in the Java rule failure messages
---
 .../maven/plugins/enforcer/RequireJavaVendor.java  |  8 +++-
 .../maven/plugins/enforcer/RequireJavaVersion.java | 11 +++++
 .../plugins/enforcer/TestRequireJavaVendor.java    | 54 ++++++++++++++--------
 .../plugins/enforcer/TestRequireJavaVersion.java   | 53 +++++++++++++--------
 4 files changed, 84 insertions(+), 42 deletions(-)

diff --git a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
index 022f7fb..5d2d62b 100644
--- a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
+++ b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVendor.java
@@ -48,13 +48,17 @@ public class RequireJavaVendor extends AbstractNonCacheableEnforcerRule {
         if (excludes != null && excludes.contains(SystemUtils.JAVA_VENDOR)) {
             String message = getMessage();
             if (message == null) {
-                message = String.format("%s is an excluded Required Java Vendor", SystemUtils.JAVA_VENDOR);
+                message = String.format(
+                        "%s is an excluded Required Java Vendor (JAVA_HOME=%s)",
+                        SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_HOME);
             }
             throw new EnforcerRuleException(message);
         } else if (includes != null && !includes.contains(SystemUtils.JAVA_VENDOR)) {
             String message = getMessage();
             if (message == null) {
-                message = String.format("%s is not an included Required Java Vendor", SystemUtils.JAVA_VENDOR);
+                message = String.format(
+                        "%s is not an included Required Java Vendor (JAVA_HOME=%s)",
+                        SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_HOME);
             }
             throw new EnforcerRuleException(message);
         }
diff --git a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVersion.java b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVersion.java
index dc8799c..87ea432 100644
--- a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVersion.java
+++ b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireJavaVersion.java
@@ -76,6 +76,8 @@ public class RequireJavaVersion extends AbstractVersionEnforcer {
                 + " Build: " + detectedJdkVersion.getBuildNumber() + " Qualifier: "
                 + detectedJdkVersion.getQualifier());
 
+        setCustomMessageIfNoneConfigured(detectedJdkVersion, getVersion());
+
         enforceVersion(helper.getLog(), "JDK", getVersion(), detectedJdkVersion);
     }
 
@@ -112,4 +114,13 @@ public class RequireJavaVersion extends AbstractVersionEnforcer {
         version = StringUtils.stripEnd(version, "-");
         return StringUtils.stripEnd(version, ".");
     }
+
+    private void setCustomMessageIfNoneConfigured(ArtifactVersion detectedJdkVersion, String allowedVersionRange) {
+        if (getMessage() == null) {
+            String message = String.format(
+                    "Detected JDK version %s (JAVA_HOME=%s) is not in the allowed range %s.",
+                    detectedJdkVersion, SystemUtils.JAVA_HOME, allowedVersionRange);
+            super.setMessage(message);
+        }
+    }
 }
diff --git a/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVendor.java b/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVendor.java
index 5050ef6..c4153dd 100644
--- a/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVendor.java
+++ b/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVendor.java
@@ -27,16 +27,14 @@ import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /**
  * The Class TestRequireJavaVendor.
  *
  * @author Tim Sijstermans
  */
-public class TestRequireJavaVendor {
+class TestRequireJavaVendor {
     private static final String NON_MATCHING_VENDOR = "non-matching-vendor";
 
     private RequireJavaVendor underTest;
@@ -47,7 +45,7 @@ public class TestRequireJavaVendor {
     }
 
     @Test
-    public void matchingInclude() throws EnforcerRuleException {
+    void matchingInclude() throws EnforcerRuleException {
         // Set the required vendor to the current system vendor
         underTest.setIncludes(Collections.singletonList(SystemUtils.JAVA_VENDOR));
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
@@ -56,25 +54,33 @@ public class TestRequireJavaVendor {
     }
 
     @Test
-    public void nonMatchingInclude() {
+    void nonMatchingInclude() {
         // Set the included vendor to something irrelevant
         underTest.setIncludes(Collections.singletonList(NON_MATCHING_VENDOR));
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
-        EnforcerRuleException e = assertThrows(EnforcerRuleException.class, () -> underTest.execute(helper));
-        assertThat(e.getMessage(), is(SystemUtils.JAVA_VENDOR + " is not an included Required Java Vendor"));
+
+        assertThatThrownBy(() -> underTest.execute(helper))
+                .isInstanceOf(EnforcerRuleException.class)
+                .hasMessage(
+                        "%s is not an included Required Java Vendor (JAVA_HOME=%s)",
+                        SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_HOME);
     }
 
     @Test
-    public void matchingExclude() {
+    void matchingExclude() {
         // Set the excluded vendor to current vendor name
         underTest.setExcludes(Collections.singletonList(SystemUtils.JAVA_VENDOR));
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
-        EnforcerRuleException e = assertThrows(EnforcerRuleException.class, () -> underTest.execute(helper));
-        assertThat(e.getMessage(), is(SystemUtils.JAVA_VENDOR + " is an excluded Required Java Vendor"));
+
+        assertThatThrownBy(() -> underTest.execute(helper))
+                .isInstanceOf(EnforcerRuleException.class)
+                .hasMessage(
+                        "%s is an excluded Required Java Vendor (JAVA_HOME=%s)",
+                        SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_HOME);
     }
 
     @Test
-    public void nonMatchingExclude() throws EnforcerRuleException {
+    void nonMatchingExclude() throws EnforcerRuleException {
         // Set the excluded vendor to something nonsensical
         underTest.setExcludes(Collections.singletonList(NON_MATCHING_VENDOR));
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
@@ -83,25 +89,33 @@ public class TestRequireJavaVendor {
     }
 
     @Test
-    public void matchingIncludeAndMatchingExclude() {
+    void matchingIncludeAndMatchingExclude() {
         underTest.setExcludes(Collections.singletonList(SystemUtils.JAVA_VENDOR));
         underTest.setIncludes(Collections.singletonList(SystemUtils.JAVA_VENDOR));
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
-        EnforcerRuleException e = assertThrows(EnforcerRuleException.class, () -> underTest.execute(helper));
-        assertThat(e.getMessage(), is(SystemUtils.JAVA_VENDOR + " is an excluded Required Java Vendor"));
+
+        assertThatThrownBy(() -> underTest.execute(helper))
+                .isInstanceOf(EnforcerRuleException.class)
+                .hasMessage(
+                        "%s is an excluded Required Java Vendor (JAVA_HOME=%s)",
+                        SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_HOME);
     }
 
     @Test
-    public void matchAnyExclude() {
+    void matchAnyExclude() {
         // Set a bunch of excluded vendors
         underTest.setExcludes(Arrays.asList(SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_VENDOR + "modified"));
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
-        EnforcerRuleException e = assertThrows(EnforcerRuleException.class, () -> underTest.execute(helper));
-        assertThat(e.getMessage(), is(SystemUtils.JAVA_VENDOR + " is an excluded Required Java Vendor"));
+
+        assertThatThrownBy(() -> underTest.execute(helper))
+                .isInstanceOf(EnforcerRuleException.class)
+                .hasMessage(
+                        "%s is an excluded Required Java Vendor (JAVA_HOME=%s)",
+                        SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_HOME);
     }
 
     @Test
-    public void matchAnyInclude() throws EnforcerRuleException {
+    void matchAnyInclude() throws EnforcerRuleException {
         // Set a bunch of included vendors
         underTest.setIncludes(Arrays.asList(SystemUtils.JAVA_VENDOR, SystemUtils.JAVA_VENDOR + "modified"));
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
@@ -110,7 +124,7 @@ public class TestRequireJavaVendor {
     }
 
     @Test
-    public void defaultRule() throws EnforcerRuleException {
+    void defaultRule() throws EnforcerRuleException {
         final EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
         underTest.execute(helper);
         // No assert and no expected exception because this test should not fail
diff --git a/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVersion.java b/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVersion.java
index 9daa091..53cfbfe 100644
--- a/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVersion.java
+++ b/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireJavaVersion.java
@@ -23,14 +23,13 @@ import java.util.stream.Stream;
 import org.apache.commons.lang3.SystemUtils;
 import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
 import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /**
  * The Class TestRequireJavaVersion.
@@ -89,30 +88,44 @@ class TestRequireJavaVersion {
 
     @Test
     void excludingTheCurrentJavaVersionViaRangeThisShouldFailWithException() {
-        assertThrows(EnforcerRuleException.class, () -> {
-            String thisVersion = RequireJavaVersion.normalizeJDKVersion(SystemUtils.JAVA_VERSION);
-
-            RequireJavaVersion rule = new RequireJavaVersion();
-            // exclude this version
-            rule.setVersion("(" + thisVersion);
-
-            EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
-            rule.execute(helper);
-            // intentionally no assertThat(...) because we expect and exception.
-        });
-        // intentionally no assertThat(...) because we expect and exception.
+        String thisVersion = RequireJavaVersion.normalizeJDKVersion(SystemUtils.JAVA_VERSION);
+        String requiredVersion = "(" + thisVersion;
+        RequireJavaVersion rule = new RequireJavaVersion();
+        rule.setVersion(requiredVersion);
+        EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
+
+        assertThatThrownBy(() -> rule.execute(helper))
+                .isInstanceOf(EnforcerRuleException.class)
+                .hasMessage("The requested JDK version %s is invalid.", requiredVersion);
     }
 
     @Test
-    @Disabled
-    // TODO: Think about the intention of this test? What should it prove?
-    void thisShouldNotCrash() throws EnforcerRuleException {
+    void shouldIncludeJavaHomeLocationInTheErrorMessage() {
+        String thisVersion = RequireJavaVersion.normalizeJDKVersion(SystemUtils.JAVA_VERSION);
+        String requiredVersion = "10000";
         RequireJavaVersion rule = new RequireJavaVersion();
-        rule.setVersion(SystemUtils.JAVA_VERSION);
+        rule.setVersion(requiredVersion);
+        EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
+
+        assertThatThrownBy(() -> rule.execute(helper))
+                .isInstanceOf(EnforcerRuleException.class)
+                .hasMessage(
+                        "Detected JDK version %s (JAVA_HOME=%s) is not in the allowed range %s.",
+                        thisVersion, SystemUtils.JAVA_HOME, requiredVersion);
+    }
 
+    @Test
+    void shouldUseCustomErrorMessage() {
+        String requiredVersion = "10000";
+        String message = "My custom error message";
+        RequireJavaVersion rule = new RequireJavaVersion();
+        rule.setVersion(requiredVersion);
+        rule.setMessage(message);
         EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
-        rule.execute(helper);
-        // intentionally no assertThat(...) because we don't expect and exception.
+
+        assertThatThrownBy(() -> rule.execute(helper))
+                .isInstanceOf(EnforcerRuleException.class)
+                .hasMessage(message);
     }
 
     /**