Skip to content

Commit

Permalink
added target/test-classes directory to check for cycles
Browse files Browse the repository at this point in the history
This enables package cycle check between test and src folder. To add
test folder the maven phase has to be changed to test-compile
  • Loading branch information
davidburkhart committed Dec 7, 2013
1 parent 8051a76 commit 696dcaf
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 71 deletions.
76 changes: 38 additions & 38 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
[![Build Status](https://buildhive.cloudbees.com/job/andrena/job/no-package-cycles-enforcer-rule/badge/icon)](https://buildhive.cloudbees.com/job/andrena/job/no-package-cycles-enforcer-rule/)

This Maven Enforcer Rule checks your project for package cycles. It fails the build if any package cycle is found, showing you the packages and classes involved in the cycle.

Usage: Add the following plugin to your POM:

```
<plugin>
<artifactId>maven-enforcer-plugin</artifactId>
<version>1.2</version>
<dependencies>
<dependency>
<groupId>de.andrena.tools.nopackagecycles</groupId>
<artifactId>no-package-cycles-enforcer-rule</artifactId>
<version>1.0.4</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>enforce-no-package-cycles</id>
<goals>
<goal>enforce</goal>
</goals>
<phase>compile</phase>
<configuration>
<rules>
<NoPackageCyclesRule implementation="de.andrena.tools.nopackagecycles.NoPackageCyclesRule" />
</rules>
</configuration>
</execution>
</executions>
</plugin>
```

See also:
* The original version by Daniel Seidewitz on [Stackoverflow](http://stackoverflow.com/questions/3416547/maven-jdepend-fail-build-with-cycles). Improved by showing all packages afflicted with cycles and the corresponding classes importing the conflicting packages.
* [JDepend](https://github.com/clarkware/jdepend), the library being used to detect package cycles.
* For more information about package cycles, see ["The Acyclic Dependencies Principle" by Robert C. Martin (Page 6)](http://www.objectmentor.com/resources/articles/granularity.pdf).
[![Build Status](https://buildhive.cloudbees.com/job/andrena/job/no-package-cycles-enforcer-rule/badge/icon)](https://buildhive.cloudbees.com/job/andrena/job/no-package-cycles-enforcer-rule/)

This Maven Enforcer Rule checks your project for package cycles. It fails the build if any package cycle is found, showing you the packages and classes involved in the cycle.

Usage: Add the following plugin to your POM:

```
<plugin>
<artifactId>maven-enforcer-plugin</artifactId>
<version>1.2</version>
<dependencies>
<dependency>
<groupId>de.andrena.tools.nopackagecycles</groupId>
<artifactId>no-package-cycles-enforcer-rule</artifactId>
<version>1.0.4</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>enforce-no-package-cycles</id>
<goals>
<goal>enforce</goal>
</goals>
<phase>test-compile</phase>
<configuration>
<rules>
<NoPackageCyclesRule implementation="de.andrena.tools.nopackagecycles.NoPackageCyclesRule" />
</rules>
</configuration>
</execution>
</executions>
</plugin>
```

See also:
* The original version by Daniel Seidewitz on [Stackoverflow](http://stackoverflow.com/questions/3416547/maven-jdepend-fail-build-with-cycles). Improved by showing all packages afflicted with cycles and the corresponding classes importing the conflicting packages.
* [JDepend](https://github.com/clarkware/jdepend), the library being used to detect package cycles.
* For more information about package cycles, see ["The Acyclic Dependencies Principle" by Robert C. Martin (Page 6)](http://www.objectmentor.com/resources/articles/granularity.pdf).
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package de.andrena.tools.nopackagecycles;

import static java.util.Collections.unmodifiableList;

import java.io.File;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;

import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;

public class DirectoriesWithClasses implements Iterable<File>{

public static final String MAVEN_PROJECT_BUILD_OUTPUT_DIRECTORY_VAR = "${project.build.outputDirectory}";
public static final String MAVEN_PROJECT_BUILD_TEST_OUTPUT_DIRECTORY_VAR = "${project.build.testOutputDirectory}";

private final List<File> directories = new LinkedList<File>();

public DirectoriesWithClasses(EnforcerRuleHelper helper) throws ExpressionEvaluationException {
addDirectoryIfExists(helper, MAVEN_PROJECT_BUILD_OUTPUT_DIRECTORY_VAR);
addDirectoryIfExists(helper, MAVEN_PROJECT_BUILD_TEST_OUTPUT_DIRECTORY_VAR);
}

private void addDirectoryIfExists(EnforcerRuleHelper helper, String variable)
throws ExpressionEvaluationException {
File directory = new File((String) helper.evaluate(variable));
if(directory.exists()) {
helper.getLog().info("Adding directory " + directory.getAbsolutePath() + " for package cycles search.");
directories.add(directory);
} else {
helper.getLog().info("Directory " + directory.getAbsolutePath() + " could not be found.");
}
}

public boolean directoriesWithClassesFound() {
return !directories.isEmpty();
}

public Iterator<File> iterator() {
return unmodifiableList(directories).iterator();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

public class NoPackageCyclesRule implements EnforcerRule {

public static final String MAVEN_PROJECT_BUILD_OUTPUT_DIRECTORY_VAR = "${project.build.outputDirectory}";

public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
try {
executePackageCycleCheckIfNecessary(helper);
Expand All @@ -29,18 +27,19 @@ public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {

private void executePackageCycleCheckIfNecessary(EnforcerRuleHelper helper) throws ExpressionEvaluationException,
IOException, EnforcerRuleException {
File classesDir = new File((String) helper.evaluate(MAVEN_PROJECT_BUILD_OUTPUT_DIRECTORY_VAR));
helper.getLog().info("Searching directory " + classesDir.getAbsolutePath() + " for package cycles.");
if (checkIsNecessary(classesDir)) {
executePackageCycleCheck(classesDir);
DirectoriesWithClasses directories = new DirectoriesWithClasses(helper);
if (directories.directoriesWithClassesFound()) {
executePackageCycleCheck(directories);
} else {
helper.getLog().info("Directory " + classesDir.getAbsolutePath() + " could not be found.");
helper.getLog().info("No directories with classes to check for cycles found.");
}
}

private void executePackageCycleCheck(File classesDir) throws IOException, EnforcerRuleException {
private void executePackageCycleCheck(Iterable<File> directories) throws IOException, EnforcerRuleException {
JDepend jdepend = createJDepend();
jdepend.addDirectory(classesDir.getAbsolutePath());
for (File directory : directories) {
jdepend.addDirectory(directory.getAbsolutePath());
}
jdepend.analyze();
if (jdepend.containsCycles()) {
throw new EnforcerRuleException("There are package cycles:" + getPackageCycles(jdepend));
Expand All @@ -57,10 +56,6 @@ private String getPackageCycles(JDepend jdepend) {
return new PackageCycleOutput(new ArrayList<JavaPackage>(packages)).getOutput();
}

private boolean checkIsNecessary(File classesDir) {
return classesDir.exists();
}

public String getCacheId() {
return "";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public void junitIntegrationTest() throws Exception {
}

private void assertPackageCycles(URL targetFolder, URL expectedOutput) throws URISyntaxException, IOException {
helper.setTargetDir(new File(targetFolder.toURI()));
helper.setTestClassesDir(new File("non-existent"));
helper.setClassesDir(new File(targetFolder.toURI()));
try {
rule.execute(helper);
fail("expected EnforcerRuleException");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public void setUp() throws Exception {
jdependMock = new JDependMock();
rule = new NoPackageCyclesRuleMock();
helper = new EnforcerRuleHelperMock();
helper.setTargetDir(temporaryFolder.getRoot());
helper.setClassesDir(temporaryFolder.newFolder("classes"));
helper.setTestClassesDir(temporaryFolder.newFolder("test-classes"));
}

@Test
Expand All @@ -64,13 +65,16 @@ public void result_IsNotValid() {

@Test
public void execute_checkNotNecessary_ClassesDirNotFound() throws Exception {
File nonExistentFolder = new File(temporaryFolder.getRoot(), "non-existent-classes-dir");
helper.setTargetDir(nonExistentFolder);
File nonExistentClassesFolder = new File(temporaryFolder.getRoot(), "non-existent-classes-dir");
helper.setClassesDir(nonExistentClassesFolder);
File nonExistentTestClassesFolder = new File(temporaryFolder.getRoot(), "non-existent-test-classes-dir");
helper.setTestClassesDir(nonExistentTestClassesFolder);
rule.execute(helper);
List<String> infoLogs = helper.getLogMock().getInfo();
assertThat(infoLogs, hasSize(2));
assertSearchingInfo(nonExistentFolder, infoLogs);
assertThat(infoLogs.get(1), is("Directory " + nonExistentFolder.getAbsolutePath() + " could not be found."));
assertThat(infoLogs, hasSize(3));
assertThat(infoLogs.get(0), is("Directory " + nonExistentClassesFolder.getAbsolutePath() + " could not be found."));
assertThat(infoLogs.get(1), is("Directory " + nonExistentTestClassesFolder.getAbsolutePath() + " could not be found."));
assertThat(infoLogs.get(2), is("No directories with classes to check for cycles found."));
}

@Test
Expand All @@ -93,8 +97,20 @@ public void execute_JdependAddDirectoryFailed_ThrowsException() throws Exception
public void execute_ContainsNoCycles() throws Exception {
rule.execute(helper);
List<String> infoLogs = helper.getLogMock().getInfo();
assertThat(infoLogs, hasSize(1));
assertSearchingInfo(temporaryFolder.getRoot(), infoLogs);
assertThat(infoLogs, hasSize(2));
assertThat(infoLogs.get(0), is("Adding directory " + new File(temporaryFolder.getRoot(), "classes").getAbsolutePath() + " for package cycles search."));
assertThat(infoLogs.get(1), is("Adding directory " + new File(temporaryFolder.getRoot(), "test-classes").getAbsolutePath() + " for package cycles search."));
}

@Test
public void execute_ContainsNoCyclesWithoutTestClasses() throws Exception {
File nonExistentTestClassesDir = new File(temporaryFolder.getRoot(), "does not exist");
helper.setTestClassesDir(nonExistentTestClassesDir);
rule.execute(helper);
List<String> infoLogs = helper.getLogMock().getInfo();
assertThat(infoLogs, hasSize(2));
assertThat(infoLogs.get(0), is("Adding directory " + new File(temporaryFolder.getRoot(), "classes").getAbsolutePath() + " for package cycles search."));
assertThat(infoLogs.get(1), is("Directory " + nonExistentTestClassesDir.getAbsolutePath() + " could not be found."));
}

@Test
Expand All @@ -104,9 +120,4 @@ public void execute_ContainsCycles() throws Exception {
expectedException.expectMessage(containsString("There are package cycles"));
rule.execute(helper);
}

private void assertSearchingInfo(File projectDirectory, List<String> infoLogs) {
assertThat(infoLogs.get(0), is("Searching directory " + projectDirectory.getAbsolutePath()
+ " for package cycles."));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
import org.codehaus.plexus.component.repository.exception.ComponentLookupException;

import de.andrena.tools.nopackagecycles.NoPackageCyclesRule;
import de.andrena.tools.nopackagecycles.DirectoriesWithClasses;

public class EnforcerRuleHelperMock implements EnforcerRuleHelper {

private File targetDir;
private File classesDir;
private File testClassesDir;
private boolean evaluateThrowsException;
private final LogMock logMock = new LogMock();

Expand All @@ -26,10 +27,14 @@ public void setEvaluateThrowsException(boolean evaluateThrowsException) {
this.evaluateThrowsException = evaluateThrowsException;
}

public void setTargetDir(File targetDir) {
this.targetDir = targetDir;
public void setClassesDir(File targetDir) {
this.classesDir = targetDir;
}

public void setTestClassesDir(File testClassesDir) {
this.testClassesDir = testClassesDir;
}

public File alignToBaseDirectory(File arg0) {
return null;
}
Expand All @@ -38,8 +43,11 @@ public Object evaluate(String variable) throws ExpressionEvaluationException {
if (evaluateThrowsException) {
throw new ExpressionEvaluationException("");
}
if (NoPackageCyclesRule.MAVEN_PROJECT_BUILD_OUTPUT_DIRECTORY_VAR.equals(variable)) {
return targetDir.getPath();
if (DirectoriesWithClasses.MAVEN_PROJECT_BUILD_OUTPUT_DIRECTORY_VAR.equals(variable)) {
return classesDir.getPath();
}
if (DirectoriesWithClasses.MAVEN_PROJECT_BUILD_TEST_OUTPUT_DIRECTORY_VAR.equals(variable)) {
return testClassesDir.getPath();
}
return null;
}
Expand Down

0 comments on commit 696dcaf

Please sign in to comment.