Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add failOnError parameter #153

Merged
merged 8 commits into from
Jun 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,12 @@ example:

### check Options

`displayFiles` default = true. Display the list of the files that are not compliant
`displayFiles` default = true. Display the list of the files that are not compliant.

`displayLimit` default = 100. Number of files to display that are not compliant`
`displayLimit` default = 100. Number of files to display that are not compliant.

`failOnError` default = true. Fail the build if non-compliant files are found.

`style` sets the formatter style to be _google_ or _aosp_. By default this is 'google'. Projects using Android conventions may prefer `aosp`.

example to not display the non-compliant files:
```xml
Expand Down Expand Up @@ -190,6 +191,29 @@ example to limit the display up to 10 files
</build>
```

example to only warn about non-compliant files instead of failing the build
```xml
<build>
<plugins>
<plugin>
<groupId>com.spotify.fmt</groupId>
<artifactId>fmt-maven-plugin</artifactId>
<version>VERSION</version>
<configuration>
<failOnError>false</failOnError>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
```

### Command line

You can also use it on the command line
Expand Down
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
30 changes: 20 additions & 10 deletions src/main/java/com/spotify/fmt/Check.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,6 +24,10 @@ public class Check extends AbstractFMT {
@Parameter(defaultValue = "100", property = "displayLimit")
private int displayLimit;

/** Fail build for non-compliant formatting */
@Parameter(defaultValue = "true", property = "fmt.failOnError")
private boolean failOnError;

/**
* Post Execute action. It is called at the end of the execute method. Subclasses can add extra
* checks.
Expand All @@ -32,29 +37,34 @@ public class Check extends AbstractFMT {
*/
@Override
protected void postExecute(FormattingResult result) throws MojoFailureException {
Consumer<String> messageConsumer = failOnError ? getLog()::error : getLog()::warn;
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"
+ (failOnError ? ", 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 (failOnError) {
throw new MojoFailureException(message);
}
}
}

Expand Down
42 changes: 42 additions & 0 deletions src/test/java/com/spotify/fmt/FMTTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -198,6 +202,7 @@ public void checkFailsWhenNotFormatted() throws Exception {
@Test
public void checkSucceedsWhenFormatted() throws Exception {
Check check = loadMojo("check_formatted", CHECK);

check.execute();
}

Expand All @@ -213,6 +218,37 @@ public void checkFailsWhenFormattingFails() throws Exception {
check.execute();
}

@Test
public void checkWarnsWhenNotFormattedAndConfiguredWithFailOnErrorFalse() throws Exception {
Check check = loadMojo("failonerrorfalse_notformatted", CHECK);
Log logSpy = setupLogSpy(check);

check.execute();

Mockito.verify(logSpy).warn(Mockito.contains("Non complying file"));
}

@Test
public void checkDoesNotWarnWhenFormattedAndConfiguredWithFailOnErrorFalse() throws Exception {
Check check = loadMojo("failonerrorfalse_formatted", CHECK);
Log logSpy = setupLogSpy(check);

check.execute();

Mockito.verify(logSpy).warn(not(Mockito.contains("Non complying file")));
}

@Test(expected = MojoFailureException.class)
public void checkFailsAndLogsErrorWhenFormattingFailsAndConfiguredWithWarningOnlyFalse()
throws Exception {
Check check = loadMojo("failonerrortrue_notformatted", CHECK);
Log logSpy = setupLogSpy(check);

check.execute();

Mockito.verify(logSpy).error(Mockito.contains("Non complying file"));
}

@SuppressWarnings("unchecked")
private <T extends AbstractFMT> T loadMojo(String pomFilePath, String goal) throws Exception {
File pomFile = loadPom(pomFilePath);
Expand All @@ -226,6 +262,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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:check
invoker.streamLogs = false
41 changes: 41 additions & 0 deletions src/test/resources/failonerrorfalse_formatted/pom.xml
Original file line number Diff line number Diff line change
@@ -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>
<failOnError>false</failOnError>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

</project>
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 file")
Original file line number Diff line number Diff line change
@@ -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 @@
invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:check
invoker.streamLogs = false
41 changes: 41 additions & 0 deletions src/test/resources/failonerrorfalse_notformatted/pom.xml
Original file line number Diff line number Diff line change
@@ -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>
<failOnError>false</failOnError>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
String buildLog = new File("${basedir}/build.log").getText("UTF-8")
// Would want to assert that it's 'WARN....Non complying', but could not get the regex to work.
assert buildLog.contains("Non complying file")
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
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 @@
invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:check
invoker.buildResult = failure
41 changes: 41 additions & 0 deletions src/test/resources/failonerrortrue_notformatted/pom.xml
Original file line number Diff line number Diff line change
@@ -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>
<failOnError>true</failOnError>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
String buildLog = new File("${basedir}/build.log").getText("UTF-8")
// Would want to assert that it's 'ERROR....Non complying', but could not get the regex to work.
assert buildLog.contains("Non complying file")
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package notestsource.src.main.java;

public
class HelloWorld1 {
public static void main(String[] args) {
System.out.println("Hello World!");;
}
}