honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Check test naming conventions on all modules 

The big win here is catching tests that are incorrectly named and will
be skipped by gradle, providing a false sense of security.

The whole thing takes about 10 seconds on my Macbook Air, not counting
compiling the test classes, which seems worth it. Because this runs as
a gradle task with propery UP-TO-DATE handling it can be skipped if the
tests haven't been changed which should save some time.

I chose to keep this in test:framework rather than a new subproject of
buildSrc because ESIntegTestCase and doesn't inroduce any additional
dependencies.
This commit is contained in:
Nik Everett 2016-02-22 01:04:58 -08:00
parent 97c4dd3b92
commit 95cc3e38fc
15 changed files with 385 additions and 191 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy
View File

@ -51,6 +51,11 @@ public class PluginBuildPlugin extends BuildPlugin {
project.integTest.clusterConfig.plugin(name, project.bundlePlugin.outputs.files)
project.tasks.run.clusterConfig.plugin(name, project.bundlePlugin.outputs.files)
}
project.namingConventions {
// Plugins decalare extensions of ESIntegTestCase as "Tests" instead of IT.
skipIntegTestInDisguise = true
}
}
createIntegTestTask(project)
createBundleTask(project)

89
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy Normal file
View File

@ -0,0 +1,89 @@
/*
* 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.gradle.precommit
import org.elasticsearch.gradle.LoggedExec
import org.elasticsearch.gradle.VersionProperties
import org.gradle.api.file.FileCollection
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.OutputFile
/**
* 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.
*/
public class NamingConventionsTask extends LoggedExec {
/**
* We use a simple "marker" file that we touch when the task succeeds
* as the task output. This is compared against the modified time of the
* inputs (ie the jars/class files).
*/
@OutputFile
File successMarker = new File(project.buildDir, 'markers/namingConventions')
/**
* The classpath to run the naming conventions checks against. Must contain the files in the test
* output directory and everything required to load those classes.
*
* We don't declare the actual test files as a dependency or input because if they change then
* this will change.
*/
@InputFiles
FileCollection classpath = project.sourceSets.test.runtimeClasspath
/**
* Should we skip the integ tests in disguise tests? Defaults to true because only core names its
* integ tests correctly.
*/
@Input
boolean skipIntegTestInDisguise = false
public NamingConventionsTask() {
dependsOn(classpath)
description = "Runs NamingConventionsCheck on ${classpath}"
executable = new File(project.javaHome, 'bin/java')
onlyIf { project.sourceSets.test.output.classesDir.exists() }
/*
* We build the arguments in a funny afterEvaluate/doFirst closure so that we can wait for the classpath to be
* ready for us. Strangely neither one on their own are good enough.
*/
project.afterEvaluate {
doFirst {
args('-cp', classpath.asPath, 'org.elasticsearch.test.NamingConventionsCheck')
if (skipIntegTestInDisguise) {
args('--skip-integ-tests-in-disguise')
}
/*
* The test framework has classes that fail the checks to validate that the checks fail properly.
* Since these would cause the build to fail we have to ignore them with this parameter. The
* process of ignoring them lets us validate that they were found so this ignore parameter acts
* as the test for the NamingConventionsCheck.
*/
if (':test:framework'.equals(project.path)) {
args('--self-test')
}
args('--', project.sourceSets.test.output.classesDir.absolutePath)
}
}
doLast { successMarker.setText("", 'UTF-8') }
}
}

8
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
View File

@ -33,6 +33,7 @@ class PrecommitTasks {
List<Task> precommitTasks = [
configureForbiddenApis(project),
configureCheckstyle(project),
configureNamingConventions(project),
project.tasks.create('forbiddenPatterns', ForbiddenPatternsTask.class),
project.tasks.create('licenseHeaders', LicenseHeadersTask.class),
project.tasks.create('jarHell', JarHellTask.class),
@ -109,4 +110,11 @@ class PrecommitTasks {
}
return checkstyleTask
}
private static Task configureNamingConventions(Project project) {
if (project.sourceSets.findByName("test")) {
return project.tasks.create('namingConventions', NamingConventionsTask)
}
return null
}
}

2
buildSrc/src/main/resources/checkstyle_suppressions.xml
View File

@ -1525,7 +1525,6 @@
<suppress files="plugins[/\\]analysis-icu[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]analysis[/\\]IcuNormalizerTokenFilterFactory.java" checks="LineLength" />
<suppress files="plugins[/\\]analysis-icu[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]analysis[/\\]IndexableBinaryStringTools.java" checks="LineLength" />
<suppress files="plugins[/\\]analysis-icu[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]analysis[/\\]AnalysisTestUtils.java" checks="LineLength" />
<suppress files="plugins[/\\]analysis-icu[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]analysis[/\\]TestIndexableBinaryStringTools.java" checks="LineLength" />
<suppress files="plugins[/\\]analysis-kuromoji[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]analysis[/\\]JapaneseStopTokenFilterFactory.java" checks="LineLength" />
<suppress files="plugins[/\\]analysis-kuromoji[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]analysis[/\\]KuromojiAnalysisTests.java" checks="LineLength" />
<suppress files="plugins[/\\]analysis-phonetic[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]analysis[/\\]PhoneticTokenFilterFactory.java" checks="LineLength" />
@ -1606,7 +1605,6 @@
<suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]cli[/\\]CheckFileCommandTests.java" checks="LineLength" />
<suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]tribe[/\\]TribeUnitTests.java" checks="LineLength" />
<suppress files="qa[/\\]smoke-test-client[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]smoketest[/\\]ESSmokeClientTestCase.java" checks="LineLength" />
<suppress files="qa[/\\]smoke-test-ingest-with-all-dependencies[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]ingest[/\\]AbstractMustacheTests.java" checks="LineLength" />
<suppress files="qa[/\\]smoke-test-ingest-with-all-dependencies[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]ingest[/\\]CombineProcessorsTests.java" checks="LineLength" />
<suppress files="qa[/\\]smoke-test-ingest-with-all-dependencies[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]ingest[/\\]IngestDocumentMustacheIT.java" checks="LineLength" />
<suppress files="qa[/\\]smoke-test-ingest-with-all-dependencies[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]ingest[/\\]IngestMustacheSetProcessorIT.java" checks="LineLength" />

177
core/src/test/java/org/elasticsearch/NamingConventionTests.java
View File

@ -1,177 +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;
import junit.framework.TestCase;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.ESTokenStreamTestCase;
import java.io.IOException;
import java.lang.reflect.Modifier;
import java.net.URISyntaxException;
import java.nio.file.FileVisitResult;
import java.nio.file.FileVisitor;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;
/**
* Simple class that ensures that all subclasses concrete of ESTestCase end with either Test | Tests
*/
public class NamingConventionTests extends ESTestCase {
// see https://github.com/elasticsearch/elasticsearch/issues/9945
public void testNamingConventions()
throws ClassNotFoundException, IOException, URISyntaxException {
final Set<Class> notImplementing = new HashSet<>();
final Set<Class> pureUnitTest = new HashSet<>();
final Set<Class> missingSuffix = new HashSet<>();
final Set<Class> integTestsInDisguise = new HashSet<>();
final Set<Class> notRunnable = new HashSet<>();
final Set<Class> innerClasses = new HashSet<>();
String[] packages = {"org.elasticsearch", "org.apache.lucene"};
for (final String packageName : packages) {
final String path = "/" + packageName.replace('.', '/');
final Path startPath = getDataPath(path);
Files.walkFileTree(startPath, new FileVisitor<Path>() {
private Path pkgPrefix = PathUtils.get(path).getParent();
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
pkgPrefix = pkgPrefix.resolve(dir.getFileName());
return FileVisitResult.CONTINUE;
}
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
try {
String filename = file.getFileName().toString();
if (filename.endsWith(".class")) {
Class<?> clazz = loadClass(filename);
if (clazz.getName().endsWith("Tests")) { // don't worry about the ones that match the pattern
if (ESIntegTestCase.class.isAssignableFrom(clazz)) {
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);
}
} else if (clazz.getName().endsWith("IT")) {
if (isTestCase(clazz) == false) {
notImplementing.add(clazz);
}
// otherwise fine
} else if (Modifier.isAbstract(clazz.getModifiers()) == false && Modifier.isInterface(clazz.getModifiers()) == false) {
if (isTestCase(clazz)) {
missingSuffix.add(clazz);
} else if (junit.framework.Test.class.isAssignableFrom(clazz)) {
pureUnitTest.add(clazz);
}
}
}
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
return FileVisitResult.CONTINUE;
}
private boolean isTestCase(Class<?> clazz) {
return LuceneTestCase.class.isAssignableFrom(clazz);
}
private Class<?> loadClass(String filename) throws ClassNotFoundException {
StringBuilder pkg = new StringBuilder();
for (Path p : pkgPrefix) {
pkg.append(p.getFileName().toString()).append(".");
}
pkg.append(filename.substring(0, filename.length() - 6));
return Thread.currentThread().getContextClassLoader().loadClass(pkg.toString());
}
@Override
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
throw exc;
}
@Override
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
pkgPrefix = pkgPrefix.getParent();
return FileVisitResult.CONTINUE;
}
});
}
assertTrue(missingSuffix.remove(WrongName.class));
assertTrue(missingSuffix.remove(WrongNameTheSecond.class));
assertTrue(notRunnable.remove(DummyAbstractTests.class));
assertTrue(notRunnable.remove(DummyInterfaceTests.class));
assertTrue(innerClasses.remove(InnerTests.class));
assertTrue(notImplementing.remove(NotImplementingTests.class));
assertTrue(pureUnitTest.remove(PlainUnit.class));
String classesToSubclass = String.join(
",",
ESTestCase.class.getSimpleName(),
ESTestCase.class.getSimpleName(),
ESTokenStreamTestCase.class.getSimpleName(),
LuceneTestCase.class.getSimpleName()
);
assertNoViolations("Not all subclasses of " + ESTestCase.class.getSimpleName() + " match the naming convention. Concrete classes must end with [Tests]:\n", missingSuffix);
assertNoViolations("Classes ending with [Tests] are abstract or interfaces:\n", notRunnable);
assertNoViolations("Found inner classes that are tests, which are excluded from the test runner:\n", innerClasses);
assertNoViolations("Pure Unit-Test found must subclass one of [" + classesToSubclass + "]:\n", pureUnitTest);
assertNoViolations("Classes ending with [Tests] must subclass [" + classesToSubclass + "]:\n", notImplementing);
assertNoViolations("Subclasses of ESIntegTestCase should end with IT as they are integration tests:\n", integTestsInDisguise);
}
private String join(Set<Class> set) {
return set.stream().map(Object::toString).collect(Collectors.joining("\n"));
}
private void assertNoViolations(String message, Set<Class> set) {
assertTrue(message + join(set), set.isEmpty());
}
/*
* Some test the test classes
*/
public static final class NotImplementingTests {}
public static final class WrongName extends ESTestCase {}
public static abstract class DummyAbstractTests extends ESTestCase {}
public interface DummyInterfaceTests {}
public static final class InnerTests extends ESTestCase {}
public static final class WrongNameTheSecond extends ESTestCase {}
public static final class PlainUnit extends TestCase {}
}

2
plugins/analysis-icu/src/test/java/org/elasticsearch/index/analysis/TestIndexableBinaryStringTools.java → plugins/analysis-icu/src/test/java/org/elasticsearch/index/analysis/IndexableBinaryStringToolsTests.java
View File

@ -40,7 +40,7 @@ import java.util.Locale;
@ThreadLeakScope(Scope.NONE)
@TimeoutSuite(millis = TimeUnits.HOUR)
@LuceneTestCase.SuppressSysoutChecks(bugUrl = "we log a lot on purpose")
public class TestIndexableBinaryStringTools extends LuceneTestCase {
public class IndexableBinaryStringToolsTests extends LuceneTestCase {
private static int NUM_RANDOM_TESTS;
private static int MAX_RANDOM_BINARY_LENGTH;
private static final String LINE_SEPARATOR = System.lineSeparator();

2
plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/AttachmentUnitTestCase.java
View File

@ -27,7 +27,7 @@ import org.elasticsearch.indices.IndicesModule;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;
public class AttachmentUnitTestCase extends ESTestCase {
public abstract class AttachmentUnitTestCase extends ESTestCase {
protected Settings testSettings;

10
qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/AbstractMustacheTests.java → qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/AbstractMustacheTestCase.java
View File

@ -32,7 +32,7 @@ import org.junit.Before;
import java.util.Collections;
public abstract class AbstractMustacheTests extends ESTestCase {
public abstract class AbstractMustacheTestCase extends ESTestCase {
protected TemplateService templateService;
@ -43,12 +43,12 @@ public abstract class AbstractMustacheTests extends ESTestCase {
.put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false)
.build();
MustacheScriptEngineService mustache = new MustacheScriptEngineService(settings);
ScriptEngineRegistry scriptEngineRegistry =
new ScriptEngineRegistry(Collections.singletonList(new ScriptEngineRegistry.ScriptEngineRegistration(MustacheScriptEngineService.class, MustacheScriptEngineService.TYPES)));
ScriptEngineRegistry scriptEngineRegistry = new ScriptEngineRegistry(Collections.singletonList(
new ScriptEngineRegistry.ScriptEngineRegistration(MustacheScriptEngineService.class, MustacheScriptEngineService.TYPES)));
ScriptContextRegistry scriptContextRegistry = new ScriptContextRegistry(Collections.emptyList());
ScriptSettings scriptSettings = new ScriptSettings(scriptEngineRegistry, scriptContextRegistry);
ScriptService scriptService =
new ScriptService(settings, new Environment(settings), Collections.singleton(mustache), null, scriptEngineRegistry, scriptContextRegistry, scriptSettings);
ScriptService scriptService = new ScriptService(settings, new Environment(settings), Collections.singleton(mustache), null,
scriptEngineRegistry, scriptContextRegistry, scriptSettings);
templateService = new InternalTemplateService(scriptService);
}

2
qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/IngestDocumentMustacheIT.java
View File

@ -31,7 +31,7 @@ import java.util.Map;
import static org.hamcrest.Matchers.equalTo;
public class IngestDocumentMustacheIT extends AbstractMustacheTests {
public class IngestDocumentMustacheIT extends AbstractMustacheTestCase {
public void testAccessMetaDataViaTemplate() {
Map<String, Object> document = new HashMap<>();

2
qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/IngestMustacheRemoveProcessorIT.java
View File

@ -26,7 +26,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
public class IngestMustacheRemoveProcessorIT extends AbstractMustacheTests {
public class IngestMustacheRemoveProcessorIT extends AbstractMustacheTestCase {
public void testRemoveProcessorMustacheExpression() throws Exception {
RemoveProcessor.Factory factory = new RemoveProcessor.Factory(templateService);

2
qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/IngestMustacheSetProcessorIT.java
View File

@ -33,7 +33,7 @@ import java.util.Map;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
public class IngestMustacheSetProcessorIT extends AbstractMustacheTests {
public class IngestMustacheSetProcessorIT extends AbstractMustacheTestCase {
public void testExpression() throws Exception {
SetProcessor processor = createSetProcessor("_index", "text {{var}}");

2
qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/TemplateServiceIT.java
View File

@ -27,7 +27,7 @@ import java.util.Map;
import static org.hamcrest.Matchers.equalTo;
public class TemplateServiceIT extends AbstractMustacheTests {
public class TemplateServiceIT extends AbstractMustacheTestCase {
public void testTemplates() {
Map<String, Object> model = new HashMap<>();

2
qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/ValueSourceMustacheIT.java
View File

@ -32,7 +32,7 @@ import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
public class ValueSourceMustacheIT extends AbstractMustacheTests {
public class ValueSourceMustacheIT extends AbstractMustacheTestCase {
public void testValueSourceWithTemplates() {
Map<String, Object> model = new HashMap<>();

221
test/framework/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java Normal file
View File

@ -0,0 +1,221 @@
/*
* 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.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.attribute.BasicFileAttributes;
import java.util.HashSet;
import java.util.Set;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;
/**
* 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.
*
* Annoyingly, this cannot be tested using standard unit tests because to do so you'd have to declare classes that violate the rules. That
* would cause the test fail which would prevent the build from passing. So we have to make a mechanism for removing those test classes. Now
* that we have such a mechanism it isn't much work to fail the process if we don't detect the offending classes. Thus, the funky
* {@code --self-test} that is only run in the test:framework project.
*/
public class NamingConventionsCheck {
public static void main(String[] args) throws IOException, ClassNotFoundException {
NamingConventionsCheck check = new NamingConventionsCheck();
boolean skipIntegTestsInDisguise = false;
boolean selfTest = false;
int i = 0;
while (true) {
switch (args[i]) {
case "--skip-integ-tests-in-disguise":
skipIntegTestsInDisguise = true;
i++;
continue;
case "--self-test":
selfTest = true;
i++;
continue;
case "--":
i++;
break;
default:
fail("Expected -- before a path.");
}
break;
}
check.check(PathUtils.get(args[i]));
if (selfTest) {
assertViolation("WrongName", check.missingSuffix);
assertViolation("WrongNameTheSecond", check.missingSuffix);
assertViolation("DummyAbstractTests", check.notRunnable);
assertViolation("DummyInterfaceTests", check.notRunnable);
assertViolation("InnerTests", check.innerClasses);
assertViolation("NotImplementingTests", check.notImplementing);
assertViolation("PlainUnit", check.pureUnitTest);
}
// Now we should have no violations
assertNoViolations("Not all subclasses of " + ESTestCase.class.getSimpleName()
+ " match the naming convention. Concrete classes must end with [Tests]", check.missingSuffix);
assertNoViolations("Classes ending with [Tests] are abstract or interfaces", check.notRunnable);
assertNoViolations("Found inner classes that are tests, which are excluded from the test runner", check.innerClasses);
String classesToSubclass = String.join(",", ESTestCase.class.getSimpleName(), ESTestCase.class.getSimpleName(),
ESTokenStreamTestCase.class.getSimpleName(), LuceneTestCase.class.getSimpleName());
assertNoViolations("Pure Unit-Test found must subclass one of [" + classesToSubclass + "]", check.pureUnitTest);
assertNoViolations("Classes ending with [Tests] must subclass [" + classesToSubclass + "]", check.notImplementing);
if (!skipIntegTestsInDisguise) {
assertNoViolations("Subclasses of ESIntegTestCase should end with IT as they are integration tests",
check.integTestsInDisguise);
}
}
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<>();
public void check(Path rootPath) throws IOException {
Files.walkFileTree(rootPath, new 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;
@Override
public 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 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 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 = loadClass(className);
if (clazz.getName().endsWith("Tests")) {
if (ESIntegTestCase.class.isAssignableFrom(clazz)) {
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);
}
} else if (clazz.getName().endsWith("IT")) {
if (isTestCase(clazz) == false) {
notImplementing.add(clazz);
}
} else if (Modifier.isAbstract(clazz.getModifiers()) == false && Modifier.isInterface(clazz.getModifiers()) == false) {
if (isTestCase(clazz)) {
missingSuffix.add(clazz);
} else if (junit.framework.Test.class.isAssignableFrom(clazz)) {
pureUnitTest.add(clazz);
}
}
}
return FileVisitResult.CONTINUE;
}
private boolean isTestCase(Class<?> clazz) {
return LuceneTestCase.class.isAssignableFrom(clazz);
}
private Class<?> loadClass(String className) {
try {
return Thread.currentThread().getContextClassLoader().loadClass(packageName + className);
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
}
@Override
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
throw exc;
}
});
}
/**
* Fail the process if there are any violations in the set. Named to look like a junit assertion even though it isn't because it is
* similar enough.
*/
@SuppressForbidden(reason = "System.err/System.exit")
private static void assertNoViolations(String message, Set<Class<?>> set) {
if (false == set.isEmpty()) {
System.err.println(message + ":");
for (Class<?> bad : set) {
System.err.println(" * " + bad.getName());
}
System.exit(1);
}
}
/**
* 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.
*/
@SuppressForbidden(reason = "System.err/System.exit")
private static void assertViolation(String className, Set<Class<?>> set) throws ClassNotFoundException {
className = "org.elasticsearch.test.test.NamingConventionsCheckBadClasses$" + className;
if (false == set.remove(Class.forName(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.
*/
@SuppressForbidden(reason = "System.err/System.exit")
private static void fail(String reason) {
System.err.println(reason);
System.exit(1);
}
}

50
test/framework/src/test/java/org/elasticsearch/test/test/NamingConventionsCheckBadClasses.java Normal file
View File

@ -0,0 +1,50 @@
/*
* 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.test;
import org.elasticsearch.test.ESTestCase;
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 ESTestCase {
}
public static abstract class DummyAbstractTests extends ESTestCase {
}
public interface DummyInterfaceTests {
}
public static final class InnerTests extends ESTestCase {
}
public static final class WrongNameTheSecond extends ESTestCase {
}
public static final class PlainUnit extends TestCase {
}
}