From df8206853bc34306fc70420a9146c00a637bc6db Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 2 Mar 2015 15:42:56 +0100 Subject: [PATCH] [TESTS] Make sure test end with ..Tests This commit adds a simple testcase that ensures all our tests end with the right naming. Closes #9945 --- .../elasticsearch/NamingConventionTests.java | 172 ++++++++++++++++++ .../common/cli/CliToolTestCase.java | 4 +- ...eMissing.java => ReplaceMissingTests.java} | 2 +- ...tion.java => LegacyVerificationTests.java} | 2 +- 4 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 src/test/java/org/elasticsearch/NamingConventionTests.java rename src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/{TestReplaceMissing.java => ReplaceMissingTests.java} (98%) rename src/test/java/org/elasticsearch/index/store/{TestLegacyVerification.java => LegacyVerificationTests.java} (98%) diff --git a/src/test/java/org/elasticsearch/NamingConventionTests.java b/src/test/java/org/elasticsearch/NamingConventionTests.java new file mode 100644 index 0000000000000..b8b7a77aac90d --- /dev/null +++ b/src/test/java/org/elasticsearch/NamingConventionTests.java @@ -0,0 +1,172 @@ +/* + * 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 com.google.common.base.Joiner; +import com.google.common.collect.Sets; +import junit.framework.TestCase; +import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.test.ElasticsearchLuceneTestCase; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.elasticsearch.test.ElasticsearchTokenStreamTestCase; +import org.junit.Ignore; +import org.junit.Test; + +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.net.URISyntaxException; +import java.nio.file.*; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.HashSet; +import java.util.Set; + +/** + * Simple class that ensures that all subclasses concrete of ElasticsearchTestCase end with either Test | Tests + */ +public class NamingConventionTests extends ElasticsearchTestCase { + + // see https://github.com/elasticsearch/elasticsearch/issues/9945 + public void testNamingConventions() + throws ClassNotFoundException, IOException, URISyntaxException { + final Set notImplementing = new HashSet<>(); + final Set pureUnitTest = new HashSet<>(); + final Set missingSuffix = new HashSet<>(); + String[] packages = {"org.elasticsearch", "org.apache.lucene"}; + for (final String packageName : packages) { + final String path = "/" + packageName.replace('.', '/'); + final Path startPath = Paths.get(NamingConventionTests.class.getResource(path).toURI()); + final Set ignore = Sets.newHashSet(Paths.get("/org/elasticsearch/stresstest"), Paths.get("/org/elasticsearch/benchmark/stress")); + Files.walkFileTree(startPath, new FileVisitor() { + private Path pkgPrefix = Paths.get(path).getParent(); + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + Path next = pkgPrefix.resolve(dir.getFileName()); + if (ignore.contains(next)) { + return FileVisitResult.SKIP_SUBTREE; + } + pkgPrefix = next; + 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 (Modifier.isAbstract(clazz.getModifiers()) == false && Modifier.isInterface(clazz.getModifiers()) == false) { + if ((clazz.getName().endsWith("Tests") || clazz.getName().endsWith("Test"))) { // don't worry about the ones that match the pattern + if (isTestCase(clazz) == false) { + notImplementing.add(clazz); + } + } else if (isTestCase(clazz)) { + missingSuffix.add(clazz); + } else if (junit.framework.Test.class.isAssignableFrom(clazz) || hasTestAnnotation(clazz)) { + pureUnitTest.add(clazz); + } + } + + } + } catch (ClassNotFoundException e) { + throw new RuntimeException(e); + } + return FileVisitResult.CONTINUE; + } + + private boolean hasTestAnnotation(Class clazz) { + for (Method method : clazz.getDeclaredMethods()) { + if (method.getAnnotation(Test.class) != null) { + return true; + } + } + return false; + + } + + private boolean isTestCase(Class clazz) { + return ElasticsearchTestCase.class.isAssignableFrom(clazz) || ElasticsearchLuceneTestCase.class.isAssignableFrom(clazz) || ElasticsearchTokenStreamTestCase.class.isAssignableFrom(clazz) || 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 Class.forName(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(notImplementing.remove(NotImplementingTests.class)); + assertTrue(notImplementing.remove(NotImplementingTest.class)); + assertTrue(pureUnitTest.remove(PlainUnit.class)); + assertTrue(pureUnitTest.remove(PlainUnitTheSecond.class)); + + String classesToSubclass = Joiner.on(',').join( + ElasticsearchTestCase.class.getSimpleName(), + ElasticsearchLuceneTestCase.class.getSimpleName(), + ElasticsearchTokenStreamTestCase.class.getSimpleName(), + LuceneTestCase.class.getSimpleName()); + assertTrue("Not all subclasses of " + ElasticsearchTestCase.class.getSimpleName() + + " match the naming convention. Concrete classes must end with [Test|Tests]: " + missingSuffix.toString(), + missingSuffix.isEmpty()); + assertTrue("Pure Unit-Test found must subclass one of [" + classesToSubclass +"] " + pureUnitTest.toString(), + pureUnitTest.isEmpty()); + assertTrue("Classes ending with Test|Tests] must subclass [" + classesToSubclass +"] " + notImplementing.toString(), + notImplementing.isEmpty()); + } + + /* + * Some test the test classes + */ + + @Ignore + public static final class NotImplementingTests {} + @Ignore + public static final class NotImplementingTest {} + + public static final class WrongName extends ElasticsearchTestCase {} + + public static final class WrongNameTheSecond extends ElasticsearchLuceneTestCase {} + + public static final class PlainUnit extends TestCase {} + + public static final class PlainUnitTheSecond { + @Test + public void foo() { + } + } + +} diff --git a/src/test/java/org/elasticsearch/common/cli/CliToolTestCase.java b/src/test/java/org/elasticsearch/common/cli/CliToolTestCase.java index 96d45bee051c5..df633b57c7b2b 100644 --- a/src/test/java/org/elasticsearch/common/cli/CliToolTestCase.java +++ b/src/test/java/org/elasticsearch/common/cli/CliToolTestCase.java @@ -22,6 +22,7 @@ import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.Strings; import org.elasticsearch.test.ElasticsearchTestCase; +import org.junit.Ignore; import java.io.IOException; import java.io.PrintWriter; @@ -33,7 +34,8 @@ /** * */ -public class CliToolTestCase extends ElasticsearchTestCase { +@Ignore +public abstract class CliToolTestCase extends ElasticsearchTestCase { protected static String[] args(String command) { if (!Strings.hasLength(command)) { diff --git a/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/TestReplaceMissing.java b/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/ReplaceMissingTests.java similarity index 98% rename from src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/TestReplaceMissing.java rename to src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/ReplaceMissingTests.java index f7583eea2e642..06d1db982a514 100644 --- a/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/TestReplaceMissing.java +++ b/src/test/java/org/elasticsearch/index/fielddata/fieldcomparator/ReplaceMissingTests.java @@ -28,7 +28,7 @@ import org.elasticsearch.test.ElasticsearchLuceneTestCase; @SuppressCodecs({ "Lucene3x", "Lucene40", "Lucene41", "Lucene42" }) // these codecs dont support missing values -public class TestReplaceMissing extends ElasticsearchLuceneTestCase { +public class ReplaceMissingTests extends ElasticsearchLuceneTestCase { public void test() throws Exception { Directory dir = newDirectory(); diff --git a/src/test/java/org/elasticsearch/index/store/TestLegacyVerification.java b/src/test/java/org/elasticsearch/index/store/LegacyVerificationTests.java similarity index 98% rename from src/test/java/org/elasticsearch/index/store/TestLegacyVerification.java rename to src/test/java/org/elasticsearch/index/store/LegacyVerificationTests.java index ee1d8b09306db..00ae1bd3c9fc8 100644 --- a/src/test/java/org/elasticsearch/index/store/TestLegacyVerification.java +++ b/src/test/java/org/elasticsearch/index/store/LegacyVerificationTests.java @@ -37,7 +37,7 @@ * segments is not longer needed. */ @Deprecated -public class TestLegacyVerification extends ElasticsearchLuceneTestCase { +public class LegacyVerificationTests extends ElasticsearchLuceneTestCase { public void testAdler32() throws Exception { Adler32 expected = new Adler32();