From b6dcd1ba1f7044ff1879efaae6fc15378e17428b Mon Sep 17 00:00:00 2001 From: Falko Modler Date: Tue, 7 Apr 2020 22:14:07 +0200 Subject: [PATCH] Fix #5: fix false negatives when checking test classes --- .../enforcer/CheckSignatureRule.java | 26 ++++++++++++++----- .../src/it/github-5-test-classes/pom.xml | 8 ++++++ .../src/test/java/localhost/Main.java | 2 ++ .../it/github-5-test-classes/verify.groovy | 4 +++ .../maven/CheckSignatureMojo.java | 19 +++++++++++--- 5 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 animal-sniffer-maven-plugin/src/it/github-5-test-classes/verify.groovy diff --git a/animal-sniffer-enforcer-rule/src/main/java/org/codehaus/mojo/animal_sniffer/enforcer/CheckSignatureRule.java b/animal-sniffer-enforcer-rule/src/main/java/org/codehaus/mojo/animal_sniffer/enforcer/CheckSignatureRule.java index 07b93fd5..c3727f0b 100644 --- a/animal-sniffer-enforcer-rule/src/main/java/org/codehaus/mojo/animal_sniffer/enforcer/CheckSignatureRule.java +++ b/animal-sniffer-enforcer-rule/src/main/java/org/codehaus/mojo/animal_sniffer/enforcer/CheckSignatureRule.java @@ -50,6 +50,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Random; @@ -185,7 +186,7 @@ public void execute( EnforcerRuleHelper helper ) MavenLogger logger = new MavenLogger( helper.getLog() ); - final Set ignoredPackages = buildPackageList( outputDirectory, project, logger ); + final Set ignoredPackages = buildPackageList( outputDirectory, testOutputDirectory, project, logger ); if ( ignores != null ) { @@ -288,18 +289,22 @@ private static Dependency findMatchingDependency( Signature signature, List buildPackageList( File outputDirectory, MavenProject project, Logger logger ) + private Set buildPackageList( File outputDirectory, File testOutputDirectory, MavenProject project, Logger logger ) throws IOException { ClassListBuilder plb = new ClassListBuilder( logger ); - apply( plb, outputDirectory, project, logger ); + apply( plb, outputDirectory, testOutputDirectory, project, logger ); return plb.getPackages(); } - private void apply( ClassFileVisitor v, File outputDirectory, MavenProject project, Logger logger ) + private void apply( ClassFileVisitor v, File outputDirectory, File testOutputDirectory, MavenProject project, Logger logger ) throws IOException { v.process( outputDirectory ); + if ( checkTestClasses ) + { + v.process( testOutputDirectory ); + } if ( ignoreDependencies ) { PatternIncludesArtifactFilter includesFilter = includeDependencies == null @@ -309,6 +314,13 @@ private void apply( ClassFileVisitor v, File outputDirectory, MavenProject proje ? null : new PatternExcludesArtifactFilter( Arrays.asList( excludeDependencies ) ); + Set classpathScopes = new HashSet<>( + Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_PROVIDED, Artifact.SCOPE_SYSTEM ) ); + if ( checkTestClasses ) + { + classpathScopes.addAll( Arrays.asList( Artifact.SCOPE_TEST, Artifact.SCOPE_RUNTIME ) ); + } + logger.debug( "Building list of classes from dependencies" ); for ( Iterator i = project.getArtifacts().iterator(); i.hasNext(); ) { @@ -321,11 +333,11 @@ private void apply( ClassFileVisitor v, File outputDirectory, MavenProject proje continue; } - if ( !( Artifact.SCOPE_COMPILE.equals( artifact.getScope() ) || Artifact.SCOPE_PROVIDED.equals( - artifact.getScope() ) || Artifact.SCOPE_SYSTEM.equals( artifact.getScope() ) ) ) + if ( !classpathScopes.contains( artifact.getScope() ) ) { logger.debug( "Skipping artifact " + artifactId( artifact ) - + " as it is not on the compile classpath." ); + + " as it is not on the " + + ( checkTestClasses ? "test" : "compile" ) + " classpath." ); continue; } diff --git a/animal-sniffer-maven-plugin/src/it/github-5-test-classes/pom.xml b/animal-sniffer-maven-plugin/src/it/github-5-test-classes/pom.xml index d101471f..bb3e4f60 100644 --- a/animal-sniffer-maven-plugin/src/it/github-5-test-classes/pom.xml +++ b/animal-sniffer-maven-plugin/src/it/github-5-test-classes/pom.xml @@ -82,4 +82,12 @@ @project.version@ + + + junit + junit + 4.13 + test + + diff --git a/animal-sniffer-maven-plugin/src/it/github-5-test-classes/src/test/java/localhost/Main.java b/animal-sniffer-maven-plugin/src/it/github-5-test-classes/src/test/java/localhost/Main.java index 9a40d6c5..1843c7c3 100644 --- a/animal-sniffer-maven-plugin/src/it/github-5-test-classes/src/test/java/localhost/Main.java +++ b/animal-sniffer-maven-plugin/src/it/github-5-test-classes/src/test/java/localhost/Main.java @@ -29,6 +29,8 @@ public class Main { public static void main( String[] args ) { + org.junit.Assert.assertTrue( true ); + if ( new java.util.concurrent.ConcurrentHashMap().isEmpty() ) { System.out.println( "All is good" ); diff --git a/animal-sniffer-maven-plugin/src/it/github-5-test-classes/verify.groovy b/animal-sniffer-maven-plugin/src/it/github-5-test-classes/verify.groovy new file mode 100644 index 00000000..85e0a468 --- /dev/null +++ b/animal-sniffer-maven-plugin/src/it/github-5-test-classes/verify.groovy @@ -0,0 +1,4 @@ +File log = new File(basedir, 'build.log') + +assert log.text.contains( 'Main.java:34: Undefined reference: java.util.concurrent.ConcurrentHashMap' ) +assert !log.text.contains( 'Undefined reference: void org.junit.Assert.assertTrue(boolean)' ) \ No newline at end of file diff --git a/animal-sniffer-maven-plugin/src/main/java/org/codehaus/mojo/animal_sniffer/maven/CheckSignatureMojo.java b/animal-sniffer-maven-plugin/src/main/java/org/codehaus/mojo/animal_sniffer/maven/CheckSignatureMojo.java index ea0e7599..15b665fc 100644 --- a/animal-sniffer-maven-plugin/src/main/java/org/codehaus/mojo/animal_sniffer/maven/CheckSignatureMojo.java +++ b/animal-sniffer-maven-plugin/src/main/java/org/codehaus/mojo/animal_sniffer/maven/CheckSignatureMojo.java @@ -52,6 +52,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Set; @@ -363,6 +364,10 @@ private void apply( ClassFileVisitor v ) throws IOException { v.process( outputDirectory ); + if ( checkTestClasses ) + { + v.process( testOutputDirectory ); + } if ( ignoreDependencies ) { PatternIncludesArtifactFilter includesFilter = includeDependencies == null @@ -373,6 +378,14 @@ private void apply( ClassFileVisitor v ) : new PatternExcludesArtifactFilter( Arrays.asList( excludeDependencies ) ); getLog().debug( "Building list of classes from dependencies" ); + + Set classpathScopes = new HashSet<>( + Arrays.asList( Artifact.SCOPE_COMPILE, Artifact.SCOPE_PROVIDED, Artifact.SCOPE_SYSTEM ) ); + if ( checkTestClasses ) + { + classpathScopes.addAll( Arrays.asList( Artifact.SCOPE_TEST, Artifact.SCOPE_RUNTIME ) ); + } + for ( Iterator i = project.getArtifacts().iterator(); i.hasNext(); ) { @@ -384,11 +397,11 @@ private void apply( ClassFileVisitor v ) continue; } - if ( !( Artifact.SCOPE_COMPILE.equals( artifact.getScope() ) || Artifact.SCOPE_PROVIDED.equals( - artifact.getScope() ) || Artifact.SCOPE_SYSTEM.equals( artifact.getScope() ) ) ) + if ( !classpathScopes.contains( artifact.getScope() ) ) { getLog().debug( "Skipping artifact " + BuildSignaturesMojo.artifactId( artifact ) - + " as it is not on the compile classpath." ); + + " as it is not on the " + + ( checkTestClasses ? "test" : "compile" ) + " classpath." ); continue; }