Author: khmarbaise Date: Sun Jun 29 12:22:33 2014 New Revision: 1606494 URL: http://svn.apache.org/r1606494 Log: [MENFORCER-174] - Follow up improved unit tests. - Fixed typos - Cleaned up docs.
Modified: maven/enforcer/trunk/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java maven/enforcer/trunk/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm maven/enforcer/trunk/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java Modified: maven/enforcer/trunk/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java URL: http://svn.apache.org/viewvc/maven/enforcer/trunk/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java?rev=1606494&r1=1606493&r2=1606494&view=diff ============================================================================== --- maven/enforcer/trunk/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java (original) +++ maven/enforcer/trunk/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanDistributionManagement.java Sun Jun 29 12:22:33 2014 @@ -28,8 +28,8 @@ import org.codehaus.plexus.component.con /** * This rule will check if a pom contains a <code>distributionManagement</code> part. This should be by best practice - * only defined once. It could happen that you like to check the parent as well which can be controlled by - * <code>ignoreParent</code> which is by default turned on. + * only defined once. It could happen that you like to check the parent as well. This can be activated by using the + * <code>ignoreParent</code> which is by default turned off (<code>true</code>) which means not to check the parent. * * @author Karl Heinz Marbaise * @since 1.4 @@ -39,7 +39,7 @@ public class BanDistributionManagement { /** - * The rule will check the parent as well. + * If we turn on the <code>ignoreParent</code> the parent will be ignored. */ private boolean ignoreParent = true; @@ -65,7 +65,6 @@ public class BanDistributionManagement private Log logger; - /** * {@inheritDoc} */ Modified: maven/enforcer/trunk/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm URL: http://svn.apache.org/viewvc/maven/enforcer/trunk/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm?rev=1606494&r1=1606493&r2=1606494&view=diff ============================================================================== --- maven/enforcer/trunk/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm (original) +++ maven/enforcer/trunk/enforcer-rules/src/site/apt/banDistributionManagement.apt.vm Sun Jun 29 12:22:33 2014 @@ -32,7 +32,7 @@ Ban Distribution Management * message - an optional message to the user if the rule fails. - * allowRepsitory - You can allow repository entry (default: false). + * allowRepository - You can allow repository entry (default: false). * allowSnapshotRepository - you can allow snapshotRepository entry (default: false). @@ -75,8 +75,7 @@ Ban Distribution Management +---+ The above configuration will prevent any declaration of distributionManagement - in your pom files at all as well in your parent of the given pom. - + in your pom files at all whereas the parent is simply ignored. Let use take a look at the following project which is a typical multi-module project. Modified: maven/enforcer/trunk/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java URL: http://svn.apache.org/viewvc/maven/enforcer/trunk/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java?rev=1606494&r1=1606493&r2=1606494&view=diff ============================================================================== --- maven/enforcer/trunk/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java (original) +++ maven/enforcer/trunk/enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/BanDistributionManagementTest.java Sun Jun 29 12:22:33 2014 @@ -30,7 +30,6 @@ import org.apache.maven.model.Site; import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException; -import org.junit.Before; import org.junit.Test; public class BanDistributionManagementTest @@ -39,44 +38,13 @@ public class BanDistributionManagementTe private EnforcerRuleHelper helper; - @Before - public void before() - throws ExpressionEvaluationException - { - } - @Test - public void shouldNotFailCauseDistributionManagementIsNotDefined() + public void shouldNotFailWithoutDistributionManagement() throws Exception { - project = mock( MavenProject.class ); - when( project.getPackaging() ).thenReturn( "jar" ); - when( project.getDistributionManagement() ).thenReturn( null ); - - helper = mock( EnforcerRuleHelper.class ); - when( helper.evaluate( "${project}" ) ).thenReturn( project ); - - when( helper.getLog() ).thenReturn( mock( Log.class ) ); - - BanDistributionManagement rule = new BanDistributionManagement(); - - rule.execute( helper ); - } - - /** - * <pre> - * <distributionManagement> - * </distributionManagement> - * </pre> - * - * TODO: Check via integration test in enforcer-plugin if we really will see this case like that. - */ - @Test - public void shouldThrowExceptionIfDistributionManagementIsDefined() - throws Exception - { - BanDistributionManagement rule = setupProjectWithDistributionManagement( null, null, null ); + BanDistributionManagement rule = setupProjectWithoutDistributionManagement(); rule.execute( helper ); + // intentionally no assert cause in case of an exception the test will be red. } /** @@ -97,6 +65,7 @@ public class BanDistributionManagementTe BanDistributionManagement rule = setupProjectWithDistributionManagement( new DeploymentRepository(), null, null ); rule.execute( helper ); + // intentionally no assert cause we expect an exception. } /** @@ -118,6 +87,7 @@ public class BanDistributionManagementTe BanDistributionManagement rule = setupProjectWithDistributionManagement( new DeploymentRepository(), new DeploymentRepository(), null ); rule.execute( helper ); + // intentionally no assert cause we expect an exception. } /** @@ -142,6 +112,7 @@ public class BanDistributionManagementTe BanDistributionManagement rule = setupProjectWithDistributionManagement( new DeploymentRepository(), new DeploymentRepository(), new Site() ); rule.execute( helper ); + // intentionally no assert cause we expect an exception. } /** @@ -161,6 +132,7 @@ public class BanDistributionManagementTe setupProjectWithDistributionManagement( new DeploymentRepository(), null, null ); rule.setAllowRepository( true ); rule.execute( helper ); + // intentionally no assert cause in case of an exception the test will be red. } /** @@ -184,6 +156,7 @@ public class BanDistributionManagementTe rule.setAllowRepository( true ); rule.setAllowSnapshotRepository( true ); rule.execute( helper ); + // intentionally no assert cause in case of an exception the test will be red. } /** @@ -211,6 +184,7 @@ public class BanDistributionManagementTe rule.setAllowSnapshotRepository( true ); rule.setAllowSite( true ); rule.execute( helper ); + // intentionally no assert cause in case of an exception the test will be red. } @Test( expected = EnforcerRuleException.class ) @@ -223,7 +197,20 @@ public class BanDistributionManagementTe rule.setIgnoreParent( false ); rule.execute( helper ); + // intentionally no assert cause we expect an exception. + } + + @Test + public void shouldThrowExceptionCauseParentProjectHasDistributionManagementSnapshotRepository() + throws Exception + { + BanDistributionManagement rule = + setupProjectWithParentDistributionManagement( null, new DeploymentRepository(), null ); + rule.setIgnoreParent( false ); + rule.setAllowSnapshotRepository( true ); + + rule.execute( helper ); } private BanDistributionManagement setupProjectWithParentDistributionManagement( DeploymentRepository repository, @@ -242,15 +229,7 @@ public class BanDistributionManagementTe when( dmParent.getSnapshotRepository() ).thenReturn( snapshotRepository ); when( dmParent.getSite() ).thenReturn( site ); - // FIXME: Remove this.... - if ( ( repository == null ) && ( snapshotRepository == null ) && ( site == null ) ) - { - when( parentProject.getDistributionManagement() ).thenReturn( null ); - } - else - { - when( parentProject.getDistributionManagement() ).thenReturn( dmParent ); - } + when( parentProject.getDistributionManagement() ).thenReturn( dmParent ); when( project.getParent() ).thenReturn( parentProject ); @@ -263,6 +242,22 @@ public class BanDistributionManagementTe return rule; } + private BanDistributionManagement setupProjectWithoutDistributionManagement() + throws ExpressionEvaluationException + { + project = mock( MavenProject.class ); + when( project.getPackaging() ).thenReturn( "jar" ); + when( project.getDistributionManagement() ).thenReturn( null ); + + helper = mock( EnforcerRuleHelper.class ); + when( helper.evaluate( "${project}" ) ).thenReturn( project ); + BanDistributionManagement rule = new BanDistributionManagement(); + + when( helper.getLog() ).thenReturn( mock( Log.class ) ); + + return rule; + } + private BanDistributionManagement setupProjectWithDistributionManagement( DeploymentRepository repository, DeploymentRepository snapshotRepository, Site site ) @@ -275,15 +270,7 @@ public class BanDistributionManagementTe when( dm.getSnapshotRepository() ).thenReturn( snapshotRepository ); when( dm.getSite() ).thenReturn( site ); - // FIXME: Remove this.... - if ( ( repository == null ) && ( snapshotRepository == null ) && ( site == null ) ) - { - when( project.getDistributionManagement() ).thenReturn( null ); - } - else - { - when( project.getDistributionManagement() ).thenReturn( dm ); - } + when( project.getDistributionManagement() ).thenReturn( dm ); helper = mock( EnforcerRuleHelper.class ); when( helper.evaluate( "${project}" ) ).thenReturn( project );