Skip to content

Commit

Permalink
Testing conventions now checks for tests in main (#37321)
Browse files Browse the repository at this point in the history
* Testing conventions now checks for tests in main

This is the last outstanding feature of the old NamingConventionsTask,
so time to remove it.

* PR review
  • Loading branch information
alpar-t committed Jan 24, 2019
1 parent b1de697 commit 082651d
Show file tree
Hide file tree
Showing 28 changed files with 66 additions and 745 deletions.
5 changes: 0 additions & 5 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,6 @@ if (project != rootProject) {
exclude '**/ForbiddenPatternsTask.java'
}

namingConventions {
testClass = 'org.elasticsearch.gradle.test.GradleUnitTestCase'
integTestClass = 'org.elasticsearch.gradle.test.GradleIntegrationTestCase'
}

testingConventions {
naming.clear()
naming {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,6 @@ public class PluginBuildPlugin extends BuildPlugin {
if (isModule == false || isXPackModule) {
addNoticeGeneration(project)
}

project.namingConventions {
// Plugins declare integration tests as "Tests" instead of IT.
skipIntegTestInDisguise = true
}
}
project.testingConventions {
naming.clear()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class PrecommitTasks {
List<Task> precommitTasks = [
configureCheckstyle(project),
configureForbiddenApisCli(project),
configureNamingConventions(project),
project.tasks.create('forbiddenPatterns', ForbiddenPatternsTask.class),
project.tasks.create('licenseHeaders', LicenseHeadersTask.class),
project.tasks.create('filepermissions', FilePermissionsTask.class),
Expand Down Expand Up @@ -230,15 +229,6 @@ class PrecommitTasks {
return checkstyleTask
}

private static Task configureNamingConventions(Project project) {
if (project.sourceSets.findByName("test")) {
Task namingConventionsTask = project.tasks.create('namingConventions', NamingConventionsTask)
namingConventionsTask.javaHome = project.compilerJavaHome
return namingConventionsTask
}
return null
}

private static Task configureLoggerUsage(Project project) {
project.configurations.create('loggerUsagePlugin')
project.dependencies.add('loggerUsagePlugin',
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.gradle.api.file.FileTree;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.SourceSetContainer;
import org.gradle.api.tasks.TaskAction;
import org.gradle.api.tasks.testing.Test;
import org.gradle.api.tasks.util.PatternFilterable;
Expand Down Expand Up @@ -122,6 +124,23 @@ public void naming(Closure<TestingConventionRule> action) {
naming.configure(action);
}

@Input
public Set<String> getMainClassNamedLikeTests() {
SourceSetContainer javaSourceSets = Boilerplate.getJavaSourceSets(getProject());
if (javaSourceSets.findByName(SourceSet.MAIN_SOURCE_SET_NAME) == null) {
// some test projects don't have a main source set
return Collections.emptySet();
}
return javaSourceSets.getByName(SourceSet.MAIN_SOURCE_SET_NAME)
.getOutput().getClassesDirs().getAsFileTree()
.getFiles().stream()
.filter(file -> file.getName().endsWith(".class"))
.map(File::getName)
.map(name -> name.substring(0, name.length() - 6))
.filter(this::implementsNamingConvention)
.collect(Collectors.toSet());
}

@TaskAction
public void doCheck() throws IOException {
final String problems;
Expand Down Expand Up @@ -235,10 +254,12 @@ public void doCheck() throws IOException {
);
}).sorted()
.collect(Collectors.joining("\n"))
)
),
// TODO: check that the testing tasks are included in the right task based on the name ( from the rule )
// TODO: check to make sure that the main source set doesn't have classes that match
// the naming convention (just the names, don't load classes)
checkNoneExists(
"Classes matching the test naming convention should be in test not main",
getMainClassNamedLikeTests()
)
);
}

Expand Down Expand Up @@ -296,6 +317,18 @@ private String checkNoneExists(String message, Stream<? extends Class<?>> stream
}
}

private String checkNoneExists(String message, Set<? extends String> candidates) {
String problem = candidates.stream()
.map(each -> " * " + each)
.sorted()
.collect(Collectors.joining("\n"));
if (problem.isEmpty() == false) {
return message + ":\n" + problem;
} else {
return "";
}
}

private String checkAtLeastOneExists(String message, Stream<? extends Class<?>> stream) {
if (stream.findAny().isPresent()) {
return "";
Expand Down Expand Up @@ -337,10 +370,14 @@ private boolean seemsLikeATest(Class<?> clazz) {
}

private boolean implementsNamingConvention(Class<?> clazz) {
return implementsNamingConvention(clazz.getName());
}

private boolean implementsNamingConvention(String className) {
if (naming.stream()
.map(TestingConventionRule::getSuffix)
.anyMatch(suffix -> clazz.getName().endsWith(suffix))) {
getLogger().debug("{} is a test because it matches the naming convention", clazz.getName());
.anyMatch(suffix -> className.endsWith(suffix))) {
getLogger().debug("{} is a test because it matches the naming convention", className);
return true;
}
return false;
Expand Down
Loading

0 comments on commit 082651d

Please sign in to comment.