diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/TestingConventionsTasks.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/TestingConventionsTasks.java index eddbe40c1a1..74d3a8dccaf 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/TestingConventionsTasks.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/TestingConventionsTasks.java @@ -44,6 +44,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.nio.file.attribute.BasicFileAttributes; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -99,25 +100,19 @@ public class TestingConventionsTasks extends DefaultTask { .collect(Collectors.toList()) ).getAsFileTree(); - final Map> classFilesPerRandomizedTestingTask = classFilesPerRandomizedTestingTask(allTestClassFiles); - final Map> classFilesPerGradleTestTask = classFilesPerGradleTestTask(); - - Map>> testClassesPerTask = - Stream.concat( - classFilesPerGradleTestTask.entrySet().stream(), - classFilesPerRandomizedTestingTask.entrySet().stream() - ) - .collect( - Collectors.toMap( - Map.Entry::getKey, - entry -> entry.getValue().stream() - .map(classes::get) - .filter(implementsNamingConvention) - .collect(Collectors.toSet()) - ) - ); - + final Map> classFilesPerTask = classFilesPerTask(allTestClassFiles); + Map>> testClassesPerTask = classFilesPerTask.entrySet().stream() + .collect( + Collectors.toMap( + Map.Entry::getKey, + entry -> entry.getValue().stream() + .map(classes::get) + .filter(implementsNamingConvention) + .collect(Collectors.toSet()) + ) + ); + problems = collectProblems( checkNoneExists( "Test classes implemented by inner classes will not run", @@ -145,16 +140,12 @@ public class TestingConventionsTasks extends DefaultTask { ), checkNoneExists( "Test classes are not included in any enabled task (" + - Stream.concat( - classFilesPerRandomizedTestingTask.keySet().stream(), - classFilesPerGradleTestTask.keySet().stream() - ).collect(Collectors.joining(",")) + ")", + classFilesPerTask.keySet().stream() + .collect(Collectors.joining(",")) + ")", allTestClassFiles.getFiles().stream() .filter(testFile -> - classFilesPerRandomizedTestingTask.values().stream() - .anyMatch(fileSet -> fileSet.contains(testFile)) == false && - classFilesPerGradleTestTask.values().stream() - .anyMatch(fileSet -> fileSet.contains(testFile)) == false + classFilesPerTask.values().stream() + .anyMatch(fileSet -> fileSet.contains(testFile)) == false ) .map(classes::get) ) @@ -179,10 +170,11 @@ public class TestingConventionsTasks extends DefaultTask { .collect(Collectors.joining()); } - @Input - public Map> classFilesPerRandomizedTestingTask(FileTree testClassFiles) { - return + public Map> classFilesPerTask(FileTree testClassFiles) { + Map> collector = new HashMap<>(); + // RandomizedTestingTask + collector.putAll( Stream.concat( getProject().getTasks().withType(getRandomizedTestingTask()).stream(), // Look at sub-projects too. As sometimes tests are implemented in parent but ran in sub-projects against @@ -191,26 +183,27 @@ public class TestingConventionsTasks extends DefaultTask { subproject.getTasks().withType(getRandomizedTestingTask()).stream() ) ) - .filter(Task::getEnabled) - .collect(Collectors.toMap( - Task::getPath, - task -> testClassFiles.matching(getRandomizedTestingPatternSet(task)).getFiles() - )); - } - - @Input - public Map> classFilesPerGradleTestTask() { - return Stream.concat( - getProject().getTasks().withType(Test.class).stream(), - getProject().getSubprojects().stream().flatMap(subproject -> - subproject.getTasks().withType(Test.class).stream() + .filter(Task::getEnabled) + .collect(Collectors.toMap( + Task::getPath, + task -> testClassFiles.matching(getRandomizedTestingPatternSet(task)).getFiles() + )) + ); + // Gradle Test + collector.putAll( + Stream.concat( + getProject().getTasks().withType(Test.class).stream(), + getProject().getSubprojects().stream().flatMap(subproject -> + subproject.getTasks().withType(Test.class).stream() + ) ) - ) - .filter(Task::getEnabled) - .collect(Collectors.toMap( - Task::getPath, - task -> task.getCandidateClassFiles().getFiles() - )); + .filter(Task::getEnabled) + .collect(Collectors.toMap( + Task::getPath, + task -> task.getCandidateClassFiles().getFiles() + )) + ); + return Collections.unmodifiableMap(collector); } @SuppressWarnings("unchecked") @@ -295,7 +288,10 @@ public class TestingConventionsTasks extends DefaultTask { } return false; } catch (NoClassDefFoundError e) { - throw new IllegalStateException("Failed to inspect class " + clazz.getName(), e); + // Include the message to get more info to get more a more useful message when running Gradle without -s + throw new IllegalStateException( + "Failed to inspect class " + clazz.getName() + ". Missing class? " + e.getMessage(), + e); } } @@ -323,9 +319,13 @@ public class TestingConventionsTasks extends DefaultTask { } private FileCollection getTestsClassPath() { - // This is doesn't need to be annotated with @Classpath because we only really care about the test source set + // Loading the classes depends on the classpath, so we could make this an input annotated with @Classpath. + // The reason we don't is that test classes are already inputs and while the dependencies are needed to load + // the classes these don't influence the checks done by this task. + // A side effect is that we could mark as up-to-date with missing dependencies, but these will be found when + // running the tests. return getProject().files( - getProject().getConfigurations().getByName("testCompile").resolve(), + getProject().getConfigurations().getByName("testRuntime").resolve(), Boilerplate.getJavaSourceSets(getProject()) .stream() .flatMap(sourceSet -> sourceSet.getOutput().getClassesDirs().getFiles().stream())