Add warningOnly parameter

spotify · May 24, 2022 · 8b21d57 · 8b21d57
1 parent b67e622
commit 8b21d57
Show file tree

Hide file tree

Showing 14 changed files with 224 additions and 10 deletions.
diff --git a/pom.xml b/pom.xml
@@ -123,6 +123,12 @@
             <version>0.28</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>4.3.1</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

diff --git a/src/main/java/com/spotify/fmt/Check.java b/src/main/java/com/spotify/fmt/Check.java
@@ -3,6 +3,7 @@
 import static java.lang.Math.max;
 import static java.lang.String.format;
 
+import java.util.function.Consumer;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugins.annotations.LifecyclePhase;
 import org.apache.maven.plugins.annotations.Mojo;
@@ -23,6 +24,10 @@ public class Check extends AbstractFMT {
   @Parameter(defaultValue = "100", property = "displayLimit")
   private int displayLimit;
 
+  /** Only show warnings instead of failing */
+  @Parameter(defaultValue = "false", property = "warningOnly")
+  private boolean warningOnly;
+
   /**
    * Post Execute action. It is called at the end of the execute method. Subclasses can add extra
    * checks.
@@ -32,29 +37,34 @@ public class Check extends AbstractFMT {
    */
   @Override
   protected void postExecute(FormattingResult result) throws MojoFailureException {
+    Consumer<String> messageConsumer = warningOnly ? getLog()::warn : getLog()::error;
     if (!result.nonComplyingFiles().isEmpty()) {
       String message =
-          "Found " + result.nonComplyingFiles().size() + " non-complying files, failing build";
-      getLog().error(message);
-      getLog()
-          .error("To fix formatting errors, run \"mvn com.spotify.fmt:fmt-maven-plugin:format\"");
+          "Found "
+              + result.nonComplyingFiles().size()
+              + " non-complying files"
+              + (warningOnly ? "" : ", failing build");
+      messageConsumer.accept(message);
+      messageConsumer.accept(
+          "To fix formatting errors, run \"mvn com.spotify.fmt:fmt-maven-plugin:format\"");
       // do not support limit < 1
       displayLimit = max(1, displayLimit);
 
       // Display first displayLimit files not formatted
       if (displayFiles) {
         result.nonComplyingFiles().stream()
             .limit(displayLimit)
-            .forEach(path -> getLog().error("Non complying file: " + path));
+            .forEach(path -> messageConsumer.accept("Non complying file: " + path));
 
         if (result.nonComplyingFiles().size() > displayLimit) {
-          getLog()
-              .error(
-                  format(
-                      "... and %d more files.", result.nonComplyingFiles().size() - displayLimit));
+          messageConsumer.accept(
+              format("... and %d more files.", result.nonComplyingFiles().size() - displayLimit));
         }
       }
-      throw new MojoFailureException(message);
+
+      if (!warningOnly) {
+        throw new MojoFailureException(message);
+      }
     }
   }
 

diff --git a/src/test/java/com/spotify/fmt/FMTTest.java b/src/test/java/com/spotify/fmt/FMTTest.java
@@ -3,14 +3,18 @@
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeTrue;
+import static org.mockito.AdditionalMatchers.not;
 
 import java.io.File;
 import java.util.List;
 import org.apache.commons.io.IOUtils;
+import org.apache.maven.plugin.Mojo;
 import org.apache.maven.plugin.MojoFailureException;
+import org.apache.maven.plugin.logging.Log;
 import org.apache.maven.plugin.testing.MojoRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class FMTTest {
   private static String FORMAT = "format";
@@ -213,6 +217,37 @@ public void checkFailsWhenFormattingFails() throws Exception {
     check.execute();
   }
 
+  @Test
+  public void checkWarnsWhenNotFormattedAndConfiguredWithWarningOnlyTrue() throws Exception {
+    Check check = loadMojo("warningonly_notformatted", CHECK);
+    Log logSpy = setupLogSpy(check);
+
+    check.execute();
+
+    Mockito.verify(logSpy).warn(Mockito.matches(".*non-complying files.*"));
+  }
+
+  @Test
+  public void checkDoesNotWarnWhenFormattedAndConfiguredWithWarningOnlyTrue() throws Exception {
+    Check check = loadMojo("warningonly_formatted", CHECK);
+    Log logSpy = setupLogSpy(check);
+
+    check.execute();
+
+    Mockito.verify(logSpy).warn(not(Mockito.matches(".*non-complying files.*")));
+  }
+
+  @Test(expected = MojoFailureException.class)
+  public void checkFailsAndLogsErrorWhenFormattingFailsAndConfiguredWithWarningOnlyFalse()
+      throws Exception {
+    Check check = loadMojo("warningonlyfalse_notformatted", CHECK);
+    Log logSpy = setupLogSpy(check);
+
+    check.execute();
+
+    Mockito.verify(logSpy).error(Mockito.matches(".*non-complying files.*"));
+  }
+
   @SuppressWarnings("unchecked")
   private <T extends AbstractFMT> T loadMojo(String pomFilePath, String goal) throws Exception {
     File pomFile = loadPom(pomFilePath);
@@ -226,6 +261,12 @@ private File loadPom(String folderName) {
     return new File("src/test/resources/", folderName);
   }
 
+  private Log setupLogSpy(Mojo mojo) {
+    Log spy = Mockito.spy(mojo.getLog());
+    mojo.setLog(spy);
+    return spy;
+  }
+
   private static boolean javaRuntimeStronglyEncapsulatesByDefault() {
     return Runtime.version().compareTo(Runtime.Version.parse("16")) >= 0;
   }

diff --git a/src/test/resources/warningonly_formatted/invoker.properties b/src/test/resources/warningonly_formatted/invoker.properties
@@ -0,0 +1,3 @@
+invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:check
+invoker.streamLogs = false
+invoker.logDirectory = logs
diff --git a/src/test/resources/warningonly_formatted/pom.xml b/src/test/resources/warningonly_formatted/pom.xml
@@ -0,0 +1,41 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>org.apache.maven.plugin.my.unit</groupId>
+    <artifactId>project-to-test</artifactId>
+    <version>1.0.0</version>
+    <packaging>jar</packaging>
+    <name>Test MyMojo</name>
+
+    <dependencies>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.13.1</version>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>com.spotify.fmt</groupId>
+                <artifactId>fmt-maven-plugin</artifactId>
+                <version>2.12</version>
+                <configuration>
+                    <warningOnly>true</warningOnly>
+                </configuration>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>
diff --git a/src/test/resources/warningonly_formatted/postbuild.groovy b/src/test/resources/warningonly_formatted/postbuild.groovy
@@ -0,0 +1,2 @@
+String buildLog = new File("${basedir}/build.log").getText("UTF-8")
+assert !buildLog.contains("non-complying files")
diff --git a/src/test/resources/warningonly_formatted/src/main/java/HelloWorld1.java b/src/test/resources/warningonly_formatted/src/main/java/HelloWorld1.java
@@ -0,0 +1,7 @@
+package notestsource.src.main.java;
+
+public class HelloWorld1 {
+  public static void main(String[] args) {
+    System.out.println("Hello World!");
+  }
+}
diff --git a/src/test/resources/warningonly_notformatted/invoker.properties b/src/test/resources/warningonly_notformatted/invoker.properties
@@ -0,0 +1,3 @@
+invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:check
+invoker.streamLogs = false
+invoker.logDirectory = logs
diff --git a/src/test/resources/warningonly_notformatted/pom.xml b/src/test/resources/warningonly_notformatted/pom.xml
@@ -0,0 +1,41 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>org.apache.maven.plugin.my.unit</groupId>
+    <artifactId>project-to-test</artifactId>
+    <version>1.0.0</version>
+    <packaging>jar</packaging>
+    <name>Test MyMojo</name>
+
+    <dependencies>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.13.1</version>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>com.spotify.fmt</groupId>
+                <artifactId>fmt-maven-plugin</artifactId>
+                <version>2.12</version>
+                <configuration>
+                    <warningOnly>true</warningOnly>
+                </configuration>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>
diff --git a/src/test/resources/warningonly_notformatted/postbuild.groovy b/src/test/resources/warningonly_notformatted/postbuild.groovy
@@ -0,0 +1,2 @@
+String buildLog = new File("${basedir}/build.log").getText("UTF-8")
+assert buildLog.contains("non-complying files")
diff --git a/src/test/resources/warningonly_notformatted/src/main/java/HelloWorld1.java b/src/test/resources/warningonly_notformatted/src/main/java/HelloWorld1.java
@@ -0,0 +1,8 @@
+package notestsource.src.main.java;
+
+public
+class HelloWorld1 {
+  public static    void main(String[] args) {
+    System.out.println("Hello World!");;
+  }
+}
diff --git a/src/test/resources/warningonlyfalse_notformatted/invoker.properties b/src/test/resources/warningonlyfalse_notformatted/invoker.properties
@@ -0,0 +1,2 @@
+invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:check
+invoker.buildResult = failure
diff --git a/src/test/resources/warningonlyfalse_notformatted/pom.xml b/src/test/resources/warningonlyfalse_notformatted/pom.xml
@@ -0,0 +1,41 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>org.apache.maven.plugin.my.unit</groupId>
+    <artifactId>project-to-test</artifactId>
+    <version>1.0.0</version>
+    <packaging>jar</packaging>
+    <name>Test MyMojo</name>
+
+    <dependencies>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.13.1</version>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>com.spotify.fmt</groupId>
+                <artifactId>fmt-maven-plugin</artifactId>
+                <version>2.12</version>
+                <configuration>
+                    <warningOnly>false</warningOnly>
+                </configuration>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>
diff --git a/src/test/resources/warningonlyfalse_notformatted/src/main/java/HelloWorld1.java b/src/test/resources/warningonlyfalse_notformatted/src/main/java/HelloWorld1.java
@@ -0,0 +1,7 @@
+package notestsource.src.main.java;
+
+public class HelloWorld1 {
+public static void main(String[] args) {
+System.out.println("Hello World!");
+}
+}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		String buildLog = new File("${basedir}/build.log").getText("UTF-8")
		assert !buildLog.contains("non-complying files")
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		String buildLog = new File("${basedir}/build.log").getText("UTF-8")
		assert buildLog.contains("non-complying files")
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:check
		invoker.buildResult = failure