Merge pull request #493 from bradleylarrick/plugin-unit-tests

Added more unit tests for japicmp-maven-plugin

Author: siom79

japicmp-maven-plugin/pom.xml

@@ -240,6 +240,11 @@
 				<targetPath>${test.directory}</targetPath>
 				<filtering>true</filtering>
 			</testResource>
+			<testResource>
+				<directory>src/test/resources/binary</directory>
+				<targetPath>${test.directory}</targetPath>
+				<filtering>false</filtering>
+			</testResource>
 		</testResources>
 	</build>
 
@@ -276,6 +281,9 @@
 			<plugin>
 				<artifactId>maven-javadoc-plugin</artifactId>
 				<configuration>
+					<sourceFileExcludes>
+						<sourceFileExclude>**/HelpMojo.java</sourceFileExclude>
+					</sourceFileExcludes>
 					<links>
 						<link>https://maven.apache.org/ref/${maven-api.version}/maven-plugin-api/apidocs</link>
 						<link>https://maven.apache.org/ref/${reporting.version}/apidocs</link>
@@ -333,6 +341,12 @@
 			</plugin>
 			<plugin>
 				<artifactId>maven-checkstyle-plugin</artifactId>
+				<configuration>
+					<!-- Explicitly specify sourceDirectory so it doesn't check generated code -->
+					<sourceDirectories>
+						<sourceDirectory>${project.basedir}/src/main</sourceDirectory>
+					</sourceDirectories>
+				</configuration>
 			</plugin>
 			<plugin>
 				<groupId>org.apache.maven.plugins</groupId>

japicmp-maven-plugin/src/main/java/japicmp/maven/ConfigParameters.java

@@ -1,6 +1,6 @@
 package japicmp.maven;
 
-import java.util.List;
+import java.util.*;
 
 /** Class for storing the japicmp configuration parameters. */
 public class ConfigParameters {
@@ -78,7 +78,6 @@ public class ConfigParameters {
 
 	private List<String> excludeModules;
 
-
 	private boolean includeExclusively = false;
 
 	private boolean excludeExclusively = false;
@@ -138,7 +137,7 @@ public boolean getBreakBuildOnBinaryIncompatibleModifications() {
 	}
 
 	void setBreakBuildOnBinaryIncompatibleModifications(
-			boolean breakBuildOnBinaryIncompatibleModifications) {
+		boolean breakBuildOnBinaryIncompatibleModifications) {
 		this.breakBuildOnBinaryIncompatibleModifications = breakBuildOnBinaryIncompatibleModifications;
 	}
 
@@ -235,7 +234,7 @@ public boolean getBreakBuildOnSourceIncompatibleModifications() {
 	}
 
 	void setBreakBuildOnSourceIncompatibleModifications(
-			boolean breakBuildOnSourceIncompatibleModifications) {
+		boolean breakBuildOnSourceIncompatibleModifications) {
 		this.breakBuildOnSourceIncompatibleModifications = breakBuildOnSourceIncompatibleModifications;
 	}
 
@@ -316,7 +315,7 @@ public List<String> getIgnoreMissingClassesByRegularExpressions() {
 	}
 
 	void setIgnoreMissingClassesByRegularExpressions(
-			List<String> ignoreMissingClassesByRegularExpressions) {
+		List<String> ignoreMissingClassesByRegularExpressions) {
 		this.ignoreMissingClassesByRegularExpressions = ignoreMissingClassesByRegularExpressions;
 	}
 
@@ -365,9 +364,9 @@ public boolean isBreakBuildBasedOnSemanticVersioningForMajorVersionZero() {
 	}
 
 	void setBreakBuildBasedOnSemanticVersioningForMajorVersionZero(
-			boolean breakBuildBasedOnSemanticVersioningForMajorVersionZero) {
+		boolean breakBuildBasedOnSemanticVersioningForMajorVersionZero) {
 		this.breakBuildBasedOnSemanticVersioningForMajorVersionZero =
-				breakBuildBasedOnSemanticVersioningForMajorVersionZero;
+			breakBuildBasedOnSemanticVersioningForMajorVersionZero;
 	}
 
 	public boolean isIncludeExlusively() {
@@ -395,7 +394,7 @@ public List<OverrideCompatibilityChangeParameter> getOverrideCompatibilityChange
 	}
 
 	void setOverrideCompatibilityChangeParameters(
-			List<OverrideCompatibilityChangeParameter> overrideCompatibilityChangeParameters) {
+		List<OverrideCompatibilityChangeParameter> overrideCompatibilityChangeParameters) {
 		this.overrideCompatibilityChangeParameters = overrideCompatibilityChangeParameters;
 	}
 
@@ -433,6 +432,10 @@ public String getCompatibilityChange() {
 			return compatibilityChange;
 		}
 
+		public void setCompatibilityChange(final String compatibilityChange) {
+			this.compatibilityChange = compatibilityChange;
+		}
+
 		public boolean isBinaryCompatible() {
 			return binaryCompatible;
 		}
@@ -444,5 +447,9 @@ public boolean isSourceCompatible() {
 		public String getSemanticVersionLevel() {
 			return semanticVersionLevel;
 		}
+
+		public void setSemanticVersionLevel(final String semanticVersionLevel) {
+			this.semanticVersionLevel = semanticVersionLevel;
+		}
 	}
 }

japicmp-maven-plugin/src/main/java/japicmp/maven/JApiCmpProcessor.java

@@ -140,13 +140,6 @@ private void setUpOverrideCompatibilityChanges(
 						break;
 					}
 				}
-				if (foundSemanticVersionLevel == null) {
-					throw new MojoFailureException("Unknown semantic version level '"
-						+ semanticVersionLevel
-						+ "'. Supported values: "
-						+ Joiner.on(',').join(
-						JApiSemanticVersionLevel.values()));
-				}
 				comparatorOptions.addOverrideCompatibilityChange(
 					new JarArchiveComparatorOptions.OverrideCompatibilityChange(foundChange,
 						configChange.isBinaryCompatible(),
@@ -760,7 +753,7 @@ private Set<Artifact> getCompileArtifacts(final MavenProject mavenProject) throw
 		Set<org.apache.maven.artifact.Artifact> projectDependencies =
 			mavenProject.getArtifacts();
 
-		HashSet<Artifact> result = new HashSet<>(1+projectDependencies.size());
+		HashSet<Artifact> result = new HashSet<>(1 + projectDependencies.size());
 		// Include the project artifact; use the reactor to resolve the project artifact in case it's not being built
 		Artifact project = RepositoryUtils.toArtifact(mavenProject.getArtifact());
 		result.add(resolveArtifact(project, ConfigurationVersion.NEW));
@@ -899,22 +892,9 @@ List<JApiCmpArchive> resolveDependencyToFile(final String parameterName,
 				}
 			}
 		} else {
-			String systemPath = dependency.getSystemPath();
-			Pattern pattern = Pattern.compile("\\$\\{([^}])");
-			Matcher matcher = pattern.matcher(systemPath);
-			if (matcher.matches()) {
-				for (int i = 1; i <= matcher.groupCount(); i++) {
-					String property = matcher.group(i);
-					String propertyResolved = mavenParameters.mavenProject().getProperties().getProperty(
-						property);
-					if (propertyResolved != null) {
-						systemPath = systemPath.replaceAll("${" + property + "}", propertyResolved);
-					} else {
-						throw new MojoFailureException("Could not resolve property '" + property + "'.");
-					}
-				}
-			}
-			File file = new File(systemPath);
+			// Substitute any properties in the path with their values
+			final String systemPath = expandProperties(dependency.getSystemPath());
+			final File file = new File(systemPath);
 			boolean addFile = true;
 			if (!file.exists()) {
 				if (ignoreMissingArtifact(configurationVersion)) {
@@ -942,6 +922,30 @@ List<JApiCmpArchive> resolveDependencyToFile(final String parameterName,
 		return jApiCmpArchives;
 	}
 
+	/**
+	 * Expands any properties found in the given {@code String}.
+	 *
+	 * @param source the source {@code String} to expand
+	 * @return the source  {@code String} with all properties expanded
+	 * @throws MojoFailureException if a property can't be resolved
+	 */
+	private String expandProperties(final String source) throws MojoFailureException {
+		final StringBuffer newString = new StringBuffer();
+		final Pattern pattern = Pattern.compile("\\$\\{([^}]*)}");
+		final Matcher matcher = pattern.matcher(source);
+		while (matcher.find()) {
+			final String property = matcher.group(1);
+			final String propertyResolved = mavenParameters.mavenProject().getProperties().getProperty(property);
+			if (propertyResolved != null) {
+				matcher.appendReplacement(newString, Matcher.quoteReplacement(propertyResolved));
+			} else {
+				throw new MojoFailureException("Could not resolve property '" + property + "'.");
+			}
+		}
+		matcher.appendTail(newString);
+		return newString.toString();
+	}
+
 	private boolean ignoreMissingArtifact(final ConfigurationVersion configurationVersion) {
 		return ignoreNonResolvableArtifacts()
 			|| ignoreMissingOldVersion(configurationVersion)

japicmp-maven-plugin/src/test/java/japicmp/maven/AbstractTest.java

@@ -1,35 +1,38 @@
 package japicmp.maven;
 
+import static java.nio.charset.Charset.defaultCharset;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.io.FileMatchers.anExistingFile;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 
-import java.io.File;
-import java.io.IOException;
-import java.nio.charset.Charset;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.List;
-import java.util.stream.Collectors;
 import org.apache.commons.io.FileUtils;
 import org.apache.maven.plugin.MojoExecution;
 import org.apache.maven.project.MavenProject;
 import org.eclipse.aether.RepositorySystem;
 import org.eclipse.aether.RepositorySystemSession;
 import org.eclipse.aether.repository.RemoteRepository;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.*;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
 abstract class AbstractTest {
 
-	final Path testDefaultDir = Paths.get("target/test-run/default/target");
+	final Path testAnnotationsDir = Paths.get("target/test-run/annotations/target");
 	final Path testConfigDir = Paths.get("target/test-run/configured/target");
+	final Path testDefaultDir = Paths.get("target/test-run/default/target");
+	final Path testMultipleDir = Paths.get("target/test-run/multiple/target");
+	final Path testOverrideDir = Paths.get("target/test-run/override/target");
 	final Path testSkipPomDir = Paths.get("target/test-run/skippom/target");
 
 	/**
@@ -39,11 +42,11 @@ abstract class AbstractTest {
 	 */
 	MavenParameters createMavenParameters() {
 		final RemoteRepository remoteRepository = new RemoteRepository.Builder("default", "releases",
-				"(https://repo.maven.apache.org/maven2").build();
+			"(https://repo.maven.apache.org/maven2").build();
 		return new MavenParameters(new ArrayList<>(), new MavenProject(),
-				mock(MojoExecution.class), "", mock(RepositorySystem.class),
-				mock(RepositorySystemSession.class),
-				Collections.singletonList(remoteRepository));
+			mock(MojoExecution.class), "", mock(RepositorySystem.class),
+			mock(RepositorySystemSession.class),
+			Collections.singletonList(remoteRepository));
 	}
 
 	/**
@@ -54,11 +57,21 @@ MavenParameters createMavenParameters() {
 	PluginParameters createPluginParameters(final ConfigParameters configParameters) {
 		final Version oldVersion = createVersion("groupId", "artifactId", "0.1.0");
 		final Version newVersion = createVersion("groupId", "artifactId", "0.1.1");
+		return createPluginParameters(configParameters, oldVersion, newVersion);
+	}
+
+	/**
+	 * Creates the PluginParameters for tests.
+	 *
+	 * @return the mocked PluginParameters
+	 */
+	PluginParameters createPluginParameters(final ConfigParameters configParameters, final Version oldVersion,
+											final Version newVersion) {
 		return new PluginParameters(false, newVersion, oldVersion, configParameters, new ArrayList<>(),
-				null,
-				null, false, new ArrayList<>(), new ArrayList<>(),
-				new ArrayList<>(), new ArrayList<>(), new SkipReport(),
-				new BreakBuild());
+			null,
+			null, false, new ArrayList<>(), new ArrayList<>(),
+			new ArrayList<>(), new ArrayList<>(), new SkipReport(),
+			new BreakBuild());
 	}
 
 	/**
@@ -67,7 +80,6 @@ PluginParameters createPluginParameters(final ConfigParameters configParameters)
 	 * @param groupId    the group ID of the Version
 	 * @param artifactId the artifact ID of the Version
 	 * @param version    the version of the Version
-	 *
 	 * @return a new Version instance
 	 */
 	Version createVersion(String groupId, String artifactId, String version) {
@@ -80,7 +92,6 @@ Version createVersion(String groupId, String artifactId, String version) {
 	 * @param groupId    the group ID of the Dependency
 	 * @param artifactId the artifact ID of the Dependency
 	 * @param version    the version of the Dependency
-	 *
 	 * @return a new Dependency instance
 	 */
 	Dependency createDependency(String groupId, String artifactId, String version) {
@@ -91,15 +102,14 @@ Dependency createDependency(String groupId, String artifactId, String version) {
 		return dependency;
 	}
 
-
 	void deleteDirectory(final Path dir) throws IOException {
 		if (Files.exists(dir)) {
 			if (!Files.isDirectory(dir)) {
 				throw new IOException(dir + " is not a directory");
 			}
 
 			// noinspection ResultOfMethodCallIgnored
-			Files.walk(dir).sorted(Comparator.reverseOrder()).map(Path :: toFile).forEach(File :: delete);
+			Files.walk(dir).sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
 		}
 	}
 
@@ -112,15 +122,45 @@ void assertFileNotExists(final File file) {
 	}
 
 	void assertFileContains(final File file, final String contents) throws IOException {
-		final String fileContents = FileUtils.readFileToString(file, Charset.defaultCharset());
-		assertThat(fileContents, containsString(contents));
+		final String fileContents = FileUtils.readFileToString(file, defaultCharset());
+		assertThat(clean(fileContents), containsString(contents));
+	}
+
+	void assertFileContainsPattern(final File file, final String contents) throws IOException {
+		Pattern pattern = Pattern.compile(contents);
+		final List<String> fileContents = FileUtils.readLines(file, defaultCharset());
+		boolean passed = false;
+		for (final String line : fileContents) {
+			if (pattern.matcher(line).matches()) {
+				passed = true;
+				break;
+			}
+		}
+		assertTrue(passed);
+	}
+
+	void assertFileNotContainsPattern(final File file, final String contents) throws IOException {
+		Pattern pattern = Pattern.compile(contents);
+		final List<String> fileContents = FileUtils.readLines(file, defaultCharset());
+		boolean passed = true;
+		for (final String line : fileContents) {
+			if (pattern.matcher(line).matches()) {
+				passed = false;
+				break;
+			}
+		}
+		assertTrue(passed);
 	}
 
 	void assertDirectoryEmpty(final Path dir) throws IOException {
 		// A non-existent directory is considered empty
 		if (Files.exists(dir)) {
 			final List<Path> contents = Files.list(dir).collect(Collectors.toList());
-			assertThat(dir.getFileName() + "is not empty", contents, is(empty()));
+			assertThat(dir.getFileName() + " is not empty", contents, is(empty()));
 		}
 	}
+
+	private String clean(final String contents) {
+		return contents.replace('\n', ' ');
+	}
 }