Testing conventions now checks for tests in main (#37321)
* 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
This commit is contained in:
parent
e7aa7e909a
commit
37768b7eac
|
@ -235,11 +235,6 @@ if (project != rootProject) {
|
|||
exclude '**/ForbiddenPatternsTask.java'
|
||||
}
|
||||
|
||||
namingConventions {
|
||||
testClass = 'org.elasticsearch.gradle.test.GradleUnitTestCase'
|
||||
integTestClass = 'org.elasticsearch.gradle.test.GradleIntegrationTestCase'
|
||||
}
|
||||
|
||||
testingConventions {
|
||||
naming.clear()
|
||||
naming {
|
||||
|
|
|
@ -72,11 +72,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()
|
||||
|
|
|
@ -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),
|
||||
|
@ -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',
|
||||
|
|
|
@ -1,167 +0,0 @@
|
|||
package org.elasticsearch.gradle.precommit;
|
||||
|
||||
import org.elasticsearch.gradle.LoggedExec;
|
||||
import org.elasticsearch.test.NamingConventionsCheck;
|
||||
import org.gradle.api.GradleException;
|
||||
import org.gradle.api.file.FileCollection;
|
||||
import org.gradle.api.plugins.JavaPluginConvention;
|
||||
import org.gradle.api.tasks.Classpath;
|
||||
import org.gradle.api.tasks.Input;
|
||||
import org.gradle.api.tasks.InputFiles;
|
||||
import org.gradle.api.tasks.SkipWhenEmpty;
|
||||
import org.gradle.api.tasks.SourceSetContainer;
|
||||
import org.gradle.api.tasks.TaskAction;
|
||||
|
||||
import java.io.File;
|
||||
import java.net.URISyntaxException;
|
||||
import java.net.URL;
|
||||
|
||||
/**
|
||||
* Runs NamingConventionsCheck on a classpath/directory combo to verify that
|
||||
* tests are named according to our conventions so they'll be picked up by
|
||||
* gradle. Read the Javadoc for NamingConventionsCheck to learn more.
|
||||
*/
|
||||
@SuppressWarnings("unchecked")
|
||||
public class NamingConventionsTask extends PrecommitTask {
|
||||
|
||||
public NamingConventionsTask() {
|
||||
setDescription("Tests that test classes aren't misnamed or misplaced");
|
||||
dependsOn(getJavaSourceSets().getByName(checkForTestsInMain ? "main" : "test").getClassesTaskName());
|
||||
}
|
||||
|
||||
@TaskAction
|
||||
public void runNamingConventions() {
|
||||
LoggedExec.javaexec(getProject(), spec -> {
|
||||
spec.classpath(
|
||||
getNamingConventionsCheckClassFiles(),
|
||||
getSourceSetClassPath()
|
||||
);
|
||||
spec.executable(getJavaHome() + "/bin/java");
|
||||
spec.jvmArgs("-Djna.nosys=true");
|
||||
spec.setMain(NamingConventionsCheck.class.getName());
|
||||
spec.args("--test-class", getTestClass());
|
||||
if (isSkipIntegTestInDisguise()) {
|
||||
spec.args("--skip-integ-tests-in-disguise");
|
||||
} else {
|
||||
spec.args("--integ-test-class", getIntegTestClass());
|
||||
}
|
||||
if (isCheckForTestsInMain()) {
|
||||
spec.args("--main");
|
||||
spec.args("--");
|
||||
} else {
|
||||
spec.args("--");
|
||||
}
|
||||
spec.args(getExistingClassesDirs().getAsPath());
|
||||
});
|
||||
}
|
||||
|
||||
@Input
|
||||
public Object getJavaHome() {
|
||||
return javaHome;
|
||||
}
|
||||
|
||||
public void setJavaHome(Object javaHome) {
|
||||
this.javaHome = javaHome;
|
||||
}
|
||||
|
||||
@Classpath
|
||||
public FileCollection getSourceSetClassPath() {
|
||||
SourceSetContainer sourceSets = getJavaSourceSets();
|
||||
return getProject().files(
|
||||
sourceSets.getByName("test").getCompileClasspath(),
|
||||
sourceSets.getByName("test").getOutput(),
|
||||
checkForTestsInMain ? sourceSets.getByName("main").getRuntimeClasspath() : getProject().files()
|
||||
);
|
||||
}
|
||||
|
||||
@InputFiles
|
||||
public File getNamingConventionsCheckClassFiles() {
|
||||
// This works because the class only depends on one class from junit that will be available from the
|
||||
// tests compile classpath. It's the most straight forward way of telling Java where to find the main
|
||||
// class.
|
||||
URL location = NamingConventionsCheck.class.getProtectionDomain().getCodeSource().getLocation();
|
||||
if (location.getProtocol().equals("file") == false) {
|
||||
throw new GradleException("Unexpected location for NamingConventionCheck class: "+ location);
|
||||
}
|
||||
try {
|
||||
return new File(location.toURI().getPath());
|
||||
} catch (URISyntaxException e) {
|
||||
throw new AssertionError(e);
|
||||
}
|
||||
}
|
||||
|
||||
@InputFiles
|
||||
@SkipWhenEmpty
|
||||
public FileCollection getExistingClassesDirs() {
|
||||
FileCollection classesDirs = getJavaSourceSets().getByName(checkForTestsInMain ? "main" : "test")
|
||||
.getOutput().getClassesDirs();
|
||||
return classesDirs.filter(it -> it.exists());
|
||||
}
|
||||
|
||||
@Input
|
||||
public boolean isSkipIntegTestInDisguise() {
|
||||
return skipIntegTestInDisguise;
|
||||
}
|
||||
|
||||
public void setSkipIntegTestInDisguise(boolean skipIntegTestInDisguise) {
|
||||
this.skipIntegTestInDisguise = skipIntegTestInDisguise;
|
||||
}
|
||||
|
||||
@Input
|
||||
public String getTestClass() {
|
||||
return testClass;
|
||||
}
|
||||
|
||||
public void setTestClass(String testClass) {
|
||||
this.testClass = testClass;
|
||||
}
|
||||
|
||||
@Input
|
||||
public String getIntegTestClass() {
|
||||
return integTestClass;
|
||||
}
|
||||
|
||||
public void setIntegTestClass(String integTestClass) {
|
||||
this.integTestClass = integTestClass;
|
||||
}
|
||||
|
||||
@Input
|
||||
public boolean isCheckForTestsInMain() {
|
||||
return checkForTestsInMain;
|
||||
}
|
||||
|
||||
public void setCheckForTestsInMain(boolean checkForTestsInMain) {
|
||||
this.checkForTestsInMain = checkForTestsInMain;
|
||||
}
|
||||
|
||||
private SourceSetContainer getJavaSourceSets() {
|
||||
return getProject().getConvention().getPlugin(JavaPluginConvention.class).getSourceSets();
|
||||
}
|
||||
|
||||
/**
|
||||
* The java home to run the check with
|
||||
*/
|
||||
private Object javaHome; // Make it an Object to allow for Groovy GString
|
||||
|
||||
/**
|
||||
* Should we skip the integ tests in disguise tests? Defaults to true because only core names its
|
||||
* integ tests correctly.
|
||||
*/
|
||||
private boolean skipIntegTestInDisguise = false;
|
||||
|
||||
/**
|
||||
* Superclass for all tests.
|
||||
*/
|
||||
private String testClass = "org.apache.lucene.util.LuceneTestCase";
|
||||
|
||||
/**
|
||||
* Superclass for all integration tests.
|
||||
*/
|
||||
private String integTestClass = "org.elasticsearch.test.ESIntegTestCase";
|
||||
|
||||
/**
|
||||
* Should the test also check the main classpath for test classes instead of
|
||||
* doing the usual checks to the test classpath.
|
||||
*/
|
||||
private boolean checkForTestsInMain = false;
|
||||
}
|
|
@ -27,6 +27,8 @@ import org.gradle.api.file.FileCollection;
|
|||
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;
|
||||
|
@ -122,6 +124,23 @@ public class TestingConventionsTasks extends DefaultTask {
|
|||
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;
|
||||
|
@ -235,10 +254,12 @@ public class TestingConventionsTasks extends DefaultTask {
|
|||
);
|
||||
}).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()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -296,6 +317,18 @@ public class TestingConventionsTasks extends DefaultTask {
|
|||
}
|
||||
}
|
||||
|
||||
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 "";
|
||||
|
@ -337,10 +370,14 @@ public class TestingConventionsTasks extends DefaultTask {
|
|||
}
|
||||
|
||||
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;
|
||||
|
|
|
@ -1,318 +0,0 @@
|
|||
/*
|
||||
* Licensed to Elasticsearch under one or more contributor
|
||||
* license agreements. See the NOTICE file distributed with
|
||||
* this work for additional information regarding copyright
|
||||
* ownership. Elasticsearch licenses this file to you under
|
||||
* the Apache License, Version 2.0 (the "License"); you may
|
||||
* not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
package org.elasticsearch.test;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.lang.reflect.Modifier;
|
||||
import java.nio.file.FileVisitResult;
|
||||
import java.nio.file.FileVisitor;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.Paths;
|
||||
import java.nio.file.attribute.BasicFileAttributes;
|
||||
import java.util.HashSet;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
/**
|
||||
* Checks that all tests in a directory are named according to our naming conventions. This is important because tests that do not follow
|
||||
* our conventions aren't run by gradle. This was once a glorious unit test but now that Elasticsearch is a multi-module project it must be
|
||||
* a class with a main method so gradle can call it for each project. This has the advantage of allowing gradle to calculate when it is
|
||||
* {@code UP-TO-DATE} so it can be skipped if the compiled classes haven't changed. This is useful on large modules for which checking all
|
||||
* the modules can be slow.
|
||||
*/
|
||||
public class NamingConventionsCheck {
|
||||
public static void main(String[] args) throws IOException {
|
||||
Class<?> testClass = null;
|
||||
Class<?> integTestClass = null;
|
||||
String rootPathList = null;
|
||||
boolean skipIntegTestsInDisguise = false;
|
||||
boolean checkMainClasses = false;
|
||||
for (int i = 0; i < args.length; i++) {
|
||||
String arg = args[i];
|
||||
switch (arg) {
|
||||
case "--test-class":
|
||||
testClass = loadClassWithoutInitializing(args[++i]);
|
||||
break;
|
||||
case "--integ-test-class":
|
||||
integTestClass = loadClassWithoutInitializing(args[++i]);
|
||||
break;
|
||||
case "--skip-integ-tests-in-disguise":
|
||||
skipIntegTestsInDisguise = true;
|
||||
break;
|
||||
case "--main":
|
||||
checkMainClasses = true;
|
||||
break;
|
||||
case "--":
|
||||
rootPathList = args[++i];
|
||||
break;
|
||||
default:
|
||||
fail("unsupported argument '" + arg + "'");
|
||||
}
|
||||
}
|
||||
if (rootPathList == null) {
|
||||
fail("No paths provided");
|
||||
return;
|
||||
}
|
||||
|
||||
NamingConventionsCheck check = new NamingConventionsCheck(testClass, integTestClass);
|
||||
for (String rootDir : rootPathList.split(Pattern.quote(File.pathSeparator))) {
|
||||
Path rootPath = Paths.get(rootDir);
|
||||
if (checkMainClasses) {
|
||||
check.checkMain(rootPath);
|
||||
} else {
|
||||
check.checkTests(rootPath, skipIntegTestsInDisguise);
|
||||
}
|
||||
}
|
||||
|
||||
// Now we should have no violations
|
||||
int exitCode = 0 ;
|
||||
exitCode += countAndPrintViolations(
|
||||
"Not all subclasses of " + check.testClass.getSimpleName()
|
||||
+ " match the naming convention. Concrete classes must end with [Tests]",
|
||||
check.missingSuffix) ;
|
||||
exitCode += countAndPrintViolations(
|
||||
"Classes ending with [Tests] are abstract or interfaces",
|
||||
check.notRunnable
|
||||
);
|
||||
exitCode += countAndPrintViolations(
|
||||
"Found inner classes that are tests, which are excluded from the test runner",
|
||||
check.innerClasses
|
||||
);
|
||||
exitCode += countAndPrintViolations(
|
||||
"Pure Unit-Test found must subclass [" + check.testClass.getSimpleName() + "]",
|
||||
check.pureUnitTest
|
||||
);
|
||||
exitCode += countAndPrintViolations(
|
||||
"Classes ending with [Tests] must subclass [" + check.testClass.getSimpleName() + "]",
|
||||
check.notImplementing
|
||||
);
|
||||
exitCode += countAndPrintViolations(
|
||||
"Classes ending with [Tests] or [IT] or extending [" +
|
||||
check.testClass.getSimpleName() + "] must be in src/test/java",
|
||||
check.testsInMain
|
||||
);
|
||||
if (skipIntegTestsInDisguise == false) {
|
||||
exitCode += countAndPrintViolations("Subclasses of " + check.integTestClass.getSimpleName() +
|
||||
" should end with IT as they are integration tests",
|
||||
check.integTestsInDisguise
|
||||
);
|
||||
}
|
||||
System.exit(exitCode);
|
||||
}
|
||||
|
||||
private final Set<Class<?>> notImplementing = new HashSet<>();
|
||||
private final Set<Class<?>> pureUnitTest = new HashSet<>();
|
||||
private final Set<Class<?>> missingSuffix = new HashSet<>();
|
||||
private final Set<Class<?>> integTestsInDisguise = new HashSet<>();
|
||||
private final Set<Class<?>> notRunnable = new HashSet<>();
|
||||
private final Set<Class<?>> innerClasses = new HashSet<>();
|
||||
private final Set<Class<?>> testsInMain = new HashSet<>();
|
||||
|
||||
private final Class<?> testClass;
|
||||
private final Class<?> integTestClass;
|
||||
|
||||
public NamingConventionsCheck(Class<?> testClass, Class<?> integTestClass) {
|
||||
this.testClass = Objects.requireNonNull(testClass, "--test-class is required");
|
||||
this.integTestClass = integTestClass;
|
||||
}
|
||||
|
||||
public void checkTests(Path rootPath, boolean skipTestsInDisguised) throws IOException {
|
||||
Files.walkFileTree(rootPath, new TestClassVisitor() {
|
||||
@Override
|
||||
protected void visitTestClass(Class<?> clazz) {
|
||||
if (skipTestsInDisguised == false &&
|
||||
integTestClass.isAssignableFrom(clazz) &&
|
||||
clazz != integTestClass) {
|
||||
integTestsInDisguise.add(clazz);
|
||||
}
|
||||
if (Modifier.isAbstract(clazz.getModifiers()) || Modifier.isInterface(clazz.getModifiers())) {
|
||||
notRunnable.add(clazz);
|
||||
} else if (isTestCase(clazz) == false) {
|
||||
notImplementing.add(clazz);
|
||||
} else if (Modifier.isStatic(clazz.getModifiers())) {
|
||||
innerClasses.add(clazz);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void visitIntegrationTestClass(Class<?> clazz) {
|
||||
if (isTestCase(clazz) == false) {
|
||||
notImplementing.add(clazz);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void visitOtherClass(Class<?> clazz) {
|
||||
if (Modifier.isAbstract(clazz.getModifiers()) || Modifier.isInterface(clazz.getModifiers())) {
|
||||
return;
|
||||
}
|
||||
if (isTestCase(clazz)) {
|
||||
missingSuffix.add(clazz);
|
||||
} else if (junit.framework.Test.class.isAssignableFrom(clazz)) {
|
||||
pureUnitTest.add(clazz);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
public void checkMain(Path rootPath) throws IOException {
|
||||
Files.walkFileTree(rootPath, new TestClassVisitor() {
|
||||
@Override
|
||||
protected void visitTestClass(Class<?> clazz) {
|
||||
testsInMain.add(clazz);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void visitIntegrationTestClass(Class<?> clazz) {
|
||||
testsInMain.add(clazz);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void visitOtherClass(Class<?> clazz) {
|
||||
if (Modifier.isAbstract(clazz.getModifiers()) || Modifier.isInterface(clazz.getModifiers())) {
|
||||
return;
|
||||
}
|
||||
if (isTestCase(clazz)) {
|
||||
testsInMain.add(clazz);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
}
|
||||
|
||||
private static int countAndPrintViolations(String message, Set<Class<?>> set) {
|
||||
if (false == set.isEmpty()) {
|
||||
System.err.println(message + ":");
|
||||
for (Class<?> bad : set) {
|
||||
System.err.println(" * " + bad.getName());
|
||||
}
|
||||
return 1;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fail the process if we didn't detect a particular violation. Named to look like a junit assertion even though it isn't because it is
|
||||
* similar enough.
|
||||
*/
|
||||
private static void assertViolation(String className, Set<Class<?>> set) {
|
||||
className = className.startsWith("org") ? className : "org.elasticsearch.test.NamingConventionsCheckBadClasses$" + className;
|
||||
if (false == set.remove(loadClassWithoutInitializing(className))) {
|
||||
System.err.println("Error in NamingConventionsCheck! Expected [" + className + "] to be a violation but wasn't.");
|
||||
System.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fail the process with the provided message.
|
||||
*/
|
||||
private static void fail(String reason) {
|
||||
System.err.println(reason);
|
||||
System.exit(1);
|
||||
}
|
||||
|
||||
static Class<?> loadClassWithoutInitializing(String name) {
|
||||
try {
|
||||
return Class.forName(name,
|
||||
// Don't initialize the class to save time. Not needed for this test and this doesn't share a VM with any other tests.
|
||||
false,
|
||||
// Use our classloader rather than the bootstrap class loader.
|
||||
NamingConventionsCheck.class.getClassLoader());
|
||||
} catch (ClassNotFoundException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
|
||||
abstract class TestClassVisitor implements FileVisitor<Path> {
|
||||
/**
|
||||
* The package name of the directory we are currently visiting. Kept as a string rather than something fancy because we load
|
||||
* just about every class and doing so requires building a string out of it anyway. At least this way we don't need to build the
|
||||
* first part of the string over and over and over again.
|
||||
*/
|
||||
private String packageName;
|
||||
|
||||
/**
|
||||
* Visit classes named like a test.
|
||||
*/
|
||||
protected abstract void visitTestClass(Class<?> clazz);
|
||||
|
||||
/**
|
||||
* Visit classes named like an integration test.
|
||||
*/
|
||||
protected abstract void visitIntegrationTestClass(Class<?> clazz);
|
||||
|
||||
/**
|
||||
* Visit classes not named like a test at all.
|
||||
*/
|
||||
protected abstract void visitOtherClass(Class<?> clazz);
|
||||
@Override
|
||||
public final FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
|
||||
// First we visit the root directory
|
||||
if (packageName == null) {
|
||||
// And it package is empty string regardless of the directory name
|
||||
packageName = "";
|
||||
} else {
|
||||
packageName += dir.getFileName() + ".";
|
||||
}
|
||||
return FileVisitResult.CONTINUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public final FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
|
||||
// Go up one package by jumping back to the second to last '.'
|
||||
packageName = packageName.substring(0, 1 + packageName.lastIndexOf('.', packageName.length() - 2));
|
||||
return FileVisitResult.CONTINUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public final FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
|
||||
String filename = file.getFileName().toString();
|
||||
if (filename.endsWith(".class")) {
|
||||
String className = filename.substring(0, filename.length() - ".class".length());
|
||||
Class<?> clazz = loadClassWithoutInitializing(packageName + className);
|
||||
if (clazz.getName().endsWith("Tests")) {
|
||||
visitTestClass(clazz);
|
||||
} else if (clazz.getName().endsWith("IT")) {
|
||||
visitIntegrationTestClass(clazz);
|
||||
} else {
|
||||
visitOtherClass(clazz);
|
||||
}
|
||||
}
|
||||
return FileVisitResult.CONTINUE;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this class a test case?
|
||||
*/
|
||||
protected boolean isTestCase(Class<?> clazz) {
|
||||
return testClass.isAssignableFrom(clazz);
|
||||
}
|
||||
|
||||
@Override
|
||||
public final FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
|
||||
throw exc;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
|
@ -1,58 +0,0 @@
|
|||
package org.elasticsearch.gradle.precommit;
|
||||
|
||||
import org.elasticsearch.gradle.test.GradleIntegrationTestCase;
|
||||
import org.gradle.testkit.runner.BuildResult;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.HashSet;
|
||||
|
||||
public class NamingConventionsTaskIT extends GradleIntegrationTestCase {
|
||||
|
||||
public void testNameCheckFailsAsItShould() {
|
||||
BuildResult result = getGradleRunner("namingConventionsSelfTest")
|
||||
.withArguments("namingConventions", "-s", "-PcheckForTestsInMain=false")
|
||||
.buildAndFail();
|
||||
|
||||
assertTaskFailed(result, ":namingConventions");
|
||||
assertOutputContains(
|
||||
result.getOutput(),
|
||||
// TODO: java9 Set.of
|
||||
new HashSet<>(
|
||||
Arrays.asList(
|
||||
"Not all subclasses of UnitTestCase match the naming convention. Concrete classes must end with [Tests]:",
|
||||
"* org.elasticsearch.test.WrongName",
|
||||
"Found inner classes that are tests, which are excluded from the test runner:",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckInMainIT$InternalInvalidTests",
|
||||
"Classes ending with [Tests] must subclass [UnitTestCase]:",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckInMainTests",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckInMainIT"
|
||||
)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
public void testNameCheckFailsAsItShouldWithMain() {
|
||||
BuildResult result = getGradleRunner("namingConventionsSelfTest")
|
||||
.withArguments("namingConventions", "-s", "-PcheckForTestsInMain=true")
|
||||
.buildAndFail();
|
||||
|
||||
assertTaskFailed(result, ":namingConventions");
|
||||
assertOutputContains(
|
||||
result.getOutput(),
|
||||
// TODO: java9 Set.of
|
||||
new HashSet<>(
|
||||
Arrays.asList(
|
||||
"Classes ending with [Tests] or [IT] or extending [UnitTestCase] must be in src/test/java:",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckBadClasses$DummyInterfaceTests",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckBadClasses$DummyInterfaceTests",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckBadClasses$DummyAbstractTests",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckBadClasses$InnerTests",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckBadClasses$NotImplementingTests",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckBadClasses$WrongNameTheSecond",
|
||||
"* org.elasticsearch.test.NamingConventionsCheckBadClasses$WrongName"
|
||||
)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
}
|
|
@ -105,4 +105,15 @@ public class TestingConventionsTasksIT extends GradleIntegrationTestCase {
|
|||
assertTaskSuccessful(result, ":valid_setup_with_base:testingConventions");
|
||||
}
|
||||
|
||||
public void testTestsInMain() {
|
||||
GradleRunner runner = getGradleRunner("testingConventions")
|
||||
.withArguments("clean", ":tests_in_main:testingConventions", "-i", "-s");
|
||||
BuildResult result = runner.buildAndFail();
|
||||
assertOutputContains(result.getOutput(),
|
||||
"Classes matching the test naming convention should be in test not main:",
|
||||
" * NamingConventionIT",
|
||||
" * NamingConventionTests"
|
||||
);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -8,7 +8,6 @@ dependenciesInfo.enabled = false
|
|||
forbiddenApisMain.enabled = false
|
||||
forbiddenApisTest.enabled = false
|
||||
thirdPartyAudit.enabled = false
|
||||
namingConventions.enabled = false
|
||||
ext.licenseFile = file("$buildDir/dummy/license")
|
||||
ext.noticeFile = file("$buildDir/dummy/notice")
|
||||
|
||||
|
|
|
@ -1,24 +0,0 @@
|
|||
plugins {
|
||||
id 'java'
|
||||
id 'elasticsearch.build'
|
||||
}
|
||||
|
||||
dependencyLicenses.enabled = false
|
||||
dependenciesInfo.enabled = false
|
||||
forbiddenApisMain.enabled = false
|
||||
forbiddenApisTest.enabled = false
|
||||
jarHell.enabled = false
|
||||
thirdPartyAudit.enabled = false
|
||||
|
||||
ext.licenseFile = file("$buildDir/dummy/license")
|
||||
ext.noticeFile = file("$buildDir/dummy/notice")
|
||||
|
||||
dependencies {
|
||||
compile "junit:junit:4.12"
|
||||
}
|
||||
|
||||
namingConventions {
|
||||
checkForTestsInMain = project.property("checkForTestsInMain") == "true"
|
||||
testClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$UnitTestCase'
|
||||
integTestClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$IntegTestCase'
|
||||
}
|
|
@ -1,62 +0,0 @@
|
|||
/*
|
||||
* Licensed to Elasticsearch under one or more contributor
|
||||
* license agreements. See the NOTICE file distributed with
|
||||
* this work for additional information regarding copyright
|
||||
* ownership. Elasticsearch licenses this file to you under
|
||||
* the Apache License, Version 2.0 (the "License"); you may
|
||||
* not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
package org.elasticsearch.test;
|
||||
|
||||
import junit.framework.TestCase;
|
||||
|
||||
/**
|
||||
* These inner classes all fail the NamingConventionsCheck. They have to live in the tests or else they won't be scanned.
|
||||
*/
|
||||
public class NamingConventionsCheckBadClasses {
|
||||
public static final class NotImplementingTests {
|
||||
}
|
||||
|
||||
public static final class WrongName extends UnitTestCase {
|
||||
/*
|
||||
* Dummy test so the tests pass. We do this *and* skip the tests so anyone who jumps back to a branch without these tests can still
|
||||
* compile without a failure. That is because clean doesn't actually clean these....
|
||||
*/
|
||||
public void testDummy() {}
|
||||
}
|
||||
|
||||
public abstract static class DummyAbstractTests extends UnitTestCase {
|
||||
}
|
||||
|
||||
public interface DummyInterfaceTests {
|
||||
}
|
||||
|
||||
public static final class InnerTests extends UnitTestCase {
|
||||
public void testDummy() {}
|
||||
}
|
||||
|
||||
public static final class WrongNameTheSecond extends UnitTestCase {
|
||||
public void testDummy() {}
|
||||
}
|
||||
|
||||
public static final class PlainUnit extends TestCase {
|
||||
public void testDummy() {}
|
||||
}
|
||||
|
||||
public abstract static class UnitTestCase extends TestCase {
|
||||
}
|
||||
|
||||
public abstract static class IntegTestCase extends UnitTestCase {
|
||||
}
|
||||
}
|
|
@ -1,31 +0,0 @@
|
|||
/*
|
||||
* Licensed to Elasticsearch under one or more contributor
|
||||
* license agreements. See the NOTICE file distributed with
|
||||
* this work for additional information regarding copyright
|
||||
* ownership. Elasticsearch licenses this file to you under
|
||||
* the Apache License, Version 2.0 (the "License"); you may
|
||||
* not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
package org.elasticsearch.test;
|
||||
|
||||
/**
|
||||
* This class should fail the naming conventions self test.
|
||||
*/
|
||||
public class NamingConventionsCheckInMainIT {
|
||||
|
||||
public static class InternalInvalidTests extends NamingConventionsCheckBadClasses.UnitTestCase {
|
||||
|
||||
}
|
||||
|
||||
}
|
|
@ -67,6 +67,10 @@ project(':valid_setup_no_base') {
|
|||
}
|
||||
}
|
||||
|
||||
project(':tests_in_main') {
|
||||
|
||||
}
|
||||
|
||||
project (':valid_setup_with_base') {
|
||||
task randomized(type: com.carrotsearch.gradle.junit4.RandomizedTestingTask) {
|
||||
include "**/*IT.class"
|
||||
|
|
|
@ -5,3 +5,4 @@ include 'all_classes_in_tasks'
|
|||
include 'not_implementing_base'
|
||||
include 'valid_setup_no_base'
|
||||
include 'valid_setup_with_base'
|
||||
include 'tests_in_main'
|
|
@ -16,11 +16,8 @@
|
|||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
package org.elasticsearch.gradle.testkit;
|
||||
|
||||
package org.elasticsearch.test;
|
||||
public class NamingConventionIT {
|
||||
|
||||
/**
|
||||
* This class should fail the naming conventions self test.
|
||||
*/
|
||||
public class WrongName extends NamingConventionsCheckBadClasses.UnitTestCase {
|
||||
}
|
|
@ -16,11 +16,8 @@
|
|||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
package org.elasticsearch.gradle.testkit;
|
||||
|
||||
package org.elasticsearch.test;
|
||||
public class NamingConventionTests {
|
||||
|
||||
/**
|
||||
* This class should fail the naming conventions self test.
|
||||
*/
|
||||
public class NamingConventionsCheckInMainTests {
|
||||
}
|
|
@ -71,12 +71,6 @@ forbiddenApisTest {
|
|||
// TODO: Not anymore. Now in :libs:core
|
||||
jarHell.enabled=false
|
||||
|
||||
namingConventions {
|
||||
testClass = 'org.elasticsearch.client.RestClientTestCase'
|
||||
//we don't have integration tests
|
||||
skipIntegTestInDisguise = true
|
||||
}
|
||||
|
||||
testingConventions {
|
||||
naming.clear()
|
||||
naming {
|
||||
|
|
|
@ -72,12 +72,6 @@ dependencyLicenses {
|
|||
// TODO: Not anymore. Now in :libs:core
|
||||
jarHell.enabled=false
|
||||
|
||||
namingConventions {
|
||||
testClass = 'org.elasticsearch.client.RestClientTestCase'
|
||||
//we don't have integration tests
|
||||
skipIntegTestInDisguise = true
|
||||
}
|
||||
|
||||
testingConventions {
|
||||
naming.clear()
|
||||
naming {
|
||||
|
|
|
@ -51,8 +51,6 @@ jarHell.enabled=false
|
|||
dependencyLicenses.enabled = false
|
||||
dependenciesInfo.enabled = false
|
||||
|
||||
namingConventions.enabled = false
|
||||
|
||||
//we aren't releasing this jar
|
||||
thirdPartyAudit.enabled = false
|
||||
unitTest.enabled = false
|
||||
|
|
|
@ -47,12 +47,6 @@ forbiddenApisTest {
|
|||
replaceSignatureFiles 'jdk-signatures', 'es-all-signatures'
|
||||
}
|
||||
|
||||
namingConventions {
|
||||
testClass = 'com.carrotsearch.randomizedtesting.RandomizedTest'
|
||||
//we don't have integration tests
|
||||
skipIntegTestInDisguise = true
|
||||
}
|
||||
|
||||
testingConventions {
|
||||
naming.clear()
|
||||
naming {
|
||||
|
|
|
@ -8,7 +8,6 @@ forbiddenApisMain {
|
|||
}
|
||||
|
||||
unitTest.enabled = false
|
||||
namingConventions.enabled = false
|
||||
javadoc.enabled = false
|
||||
loggerUsageCheck.enabled = false
|
||||
jarHell.enabled = false
|
||||
|
|
|
@ -33,11 +33,6 @@ tasks.withType(CheckForbiddenApis) {
|
|||
replaceSignatureFiles 'jdk-signatures'
|
||||
}
|
||||
|
||||
namingConventions {
|
||||
testClass = 'org.elasticsearch.tools.launchers.LaunchersTestCase'
|
||||
skipIntegTestInDisguise = true
|
||||
}
|
||||
|
||||
testingConventions {
|
||||
naming.clear()
|
||||
naming {
|
||||
|
|
|
@ -63,10 +63,6 @@ if (isEclipse) {
|
|||
// JAR hell is part of core which we do not want to add as a dependency
|
||||
jarHell.enabled = false
|
||||
|
||||
namingConventions {
|
||||
testClass = 'junit.framework.TestCase'
|
||||
}
|
||||
|
||||
testingConventions {
|
||||
naming.clear()
|
||||
naming {
|
||||
|
|
|
@ -72,6 +72,8 @@ forbiddenApisMain {
|
|||
|
||||
// we don't have additional tests for the tests themselves
|
||||
tasks.unitTest.enabled = false
|
||||
// Tests are destructive and meant to run in a VM, they don't adhere to general conventions
|
||||
testingConventions.enabled = false
|
||||
|
||||
// this project doesn't get published
|
||||
tasks.dependencyLicenses.enabled = false
|
||||
|
|
|
@ -64,12 +64,6 @@ thirdPartyAudit.ignoreMissingClasses (
|
|||
'org.jmock.core.Constraint'
|
||||
)
|
||||
|
||||
task namingConventionsMain(type: org.elasticsearch.gradle.precommit.NamingConventionsTask) {
|
||||
checkForTestsInMain = true
|
||||
javaHome = project.compilerJavaHome
|
||||
}
|
||||
precommit.dependsOn namingConventionsMain
|
||||
|
||||
unitTest {
|
||||
systemProperty 'tests.gradle_index_compat_versions', bwcVersions.indexCompatible.join(',')
|
||||
systemProperty 'tests.gradle_wire_compat_versions', bwcVersions.wireCompatible.join(',')
|
||||
|
|
|
@ -29,8 +29,3 @@ if (project.rootProject.vagrantSupported) {
|
|||
unitTest.enabled = false
|
||||
testingConventions.enabled = false
|
||||
}
|
||||
|
||||
namingConventions {
|
||||
// integ tests use Tests instead of IT
|
||||
skipIntegTestInDisguise = true
|
||||
}
|
||||
|
|
|
@ -23,12 +23,6 @@ forbiddenApisTest {
|
|||
replaceSignatureFiles 'jdk-signatures', 'es-all-signatures'
|
||||
}
|
||||
|
||||
namingConventions {
|
||||
testClass = 'com.carrotsearch.randomizedtesting.RandomizedTest'
|
||||
//we don't have integration tests
|
||||
skipIntegTestInDisguise = true
|
||||
}
|
||||
|
||||
testingConventions {
|
||||
naming.clear()
|
||||
naming {
|
||||
|
|
Loading…
Reference in New Issue