From c4ff575b8972e6612de6b52de1a822943b54b1db Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Wed, 28 Nov 2012 18:58:24 +0000 Subject: [PATCH] HBASE-7229 ClassFinder finds compat tests during mvn package, causing TestCheckTestClasses to fail git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1414864 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/hadoop/hbase/ClassFinder.java | 36 ++++++-- .../apache/hadoop/hbase/ClassTestFinder.java | 11 ++- .../apache/hadoop/hbase/TestClassFinder.java | 90 ++++++++++++------- 3 files changed, 95 insertions(+), 42 deletions(-) diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java index 2461534d65b..88cd1695a83 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java @@ -46,10 +46,15 @@ public class ClassFinder { private static final Log LOG = LogFactory.getLog(ClassFinder.class); private static String CLASS_EXT = ".class"; + private ResourcePathFilter resourcePathFilter; private FileNameFilter fileNameFilter; private ClassFilter classFilter; private FileFilter fileFilter; + public static interface ResourcePathFilter { + public boolean isCandidatePath(String resourcePath, boolean isJar); + }; + public static interface FileNameFilter { public boolean isCandidateFile(String fileName, String absFilePath); }; @@ -58,7 +63,13 @@ public class ClassFinder { public boolean isCandidateClass(Class c); }; - public ClassFinder(FileNameFilter fileNameFilter, ClassFilter classFilter) { + public ClassFinder() { + this(null, null, null); + } + + public ClassFinder(ResourcePathFilter resourcePathFilter, + FileNameFilter fileNameFilter, ClassFilter classFilter) { + this.resourcePathFilter = resourcePathFilter; this.classFilter = classFilter; this.fileNameFilter = fileNameFilter; this.fileFilter = new FileFilterWithName(fileNameFilter); @@ -93,10 +104,16 @@ public class ClassFinder { URL resource = resources.nextElement(); String resourcePath = resource.getFile(); Matcher matcher = jarResourceRe.matcher(resourcePath); - if (matcher.find()) { - jars.add(matcher.group(1)); - } else { - dirs.add(new File(resource.getFile())); + boolean isJar = matcher.find(); + resourcePath = isJar ? matcher.group(1) : resourcePath; + if (null == this.resourcePathFilter + || this.resourcePathFilter.isCandidatePath(resourcePath, isJar)) { + LOG.debug("Will look for classes in " + resourcePath); + if (isJar) { + jars.add(resourcePath); + } else { + dirs.add(new File(resourcePath)); + } } } @@ -145,7 +162,8 @@ public class ClassFinder { } int ix = className.lastIndexOf('/'); String fileName = (ix >= 0) ? className.substring(ix + 1) : className; - if (!this.fileNameFilter.isCandidateFile(fileName, className)) { + if (null != this.fileNameFilter + && !this.fileNameFilter.isCandidateFile(fileName, className)) { continue; } className = className @@ -200,7 +218,8 @@ public class ClassFinder { throws ClassNotFoundException, LinkageError { try { Class c = Class.forName(className, false, this.getClass().getClassLoader()); - return classFilter.isCandidateClass(c) ? c : null; + boolean isCandidateClass = null == classFilter || classFilter.isCandidateClass(c); + return isCandidateClass ? c : null; } catch (ClassNotFoundException classNotFoundEx) { if (!proceedOnExceptions) { throw classNotFoundEx; @@ -226,7 +245,8 @@ public class ClassFinder { public boolean accept(File file) { return file.isDirectory() || (file.getName().endsWith(CLASS_EXT) - && nameFilter.isCandidateFile(file.getName(), file.getAbsolutePath())); + && (null == nameFilter + || nameFilter.isCandidateFile(file.getName(), file.getAbsolutePath()))); } }; }; diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java index 0fefcd57313..b355a2b55e8 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java @@ -35,11 +35,11 @@ import org.junit.runners.Suite; public class ClassTestFinder extends ClassFinder { public ClassTestFinder() { - super(new TestFileNameFilter(), new TestClassFilter()); + super(new TestFileNameFilter(), new TestFileNameFilter(), new TestClassFilter()); } public ClassTestFinder(Class category) { - super(new TestFileNameFilter(), new TestClassFilter(category)); + super(new TestFileNameFilter(), new TestFileNameFilter(), new TestClassFilter(category)); } public static Class[] getCategoryAnnotations(Class c) { @@ -50,7 +50,7 @@ public class ClassTestFinder extends ClassFinder { return new Class[0]; } - private static class TestFileNameFilter implements FileNameFilter { + private static class TestFileNameFilter implements FileNameFilter, ResourcePathFilter { private static final Pattern hadoopCompactRe = Pattern.compile("hbase-hadoop\\d?-compat"); @@ -60,6 +60,11 @@ public class ClassTestFinder extends ClassFinder { || fileName.startsWith("IntegrationTest"); return isTestFile && !hadoopCompactRe.matcher(absFilePath).find(); } + + @Override + public boolean isCandidatePath(String resourcePath, boolean isJar) { + return !hadoopCompactRe.matcher(resourcePath).find(); + } }; /* diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestClassFinder.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestClassFinder.java index c6c77ae7ef9..876ea1d6a5d 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestClassFinder.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestClassFinder.java @@ -41,9 +41,12 @@ import org.junit.BeforeClass; import org.junit.Test; import org.apache.commons.io.FileUtils; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; @Category(SmallTests.class) public class TestClassFinder { + private static final Log LOG = LogFactory.getLog(TestClassFinder.class); private static final HBaseCommonTestingUtility testUtil = new HBaseCommonTestingUtility(); private static final String BASEPKG = "tfcpkg"; @@ -53,25 +56,8 @@ public class TestClassFinder { private static AtomicLong testCounter = new AtomicLong(0); private static AtomicLong jarCounter = new AtomicLong(0); - private static String basePath = null; - // Default name/class filters for testing. - private static final ClassFinder.FileNameFilter trueNameFilter = - new ClassFinder.FileNameFilter() { - @Override - public boolean isCandidateFile(String fileName, String absFilePath) { - return true; - } - }; - private static final ClassFinder.ClassFilter trueClassFilter = - new ClassFinder.ClassFilter() { - @Override - public boolean isCandidateClass(Class c) { - return true; - } - }; - @BeforeClass public static void createTestDir() throws IOException { basePath = testUtil.getDataTestDir(TestClassFinder.class.getSimpleName()).toString(); @@ -100,7 +86,7 @@ public class TestClassFinder { packageAndLoadJar(c1, c3); packageAndLoadJar(c2); - ClassFinder allClassesFinder = new ClassFinder(trueNameFilter, trueClassFilter); + ClassFinder allClassesFinder = new ClassFinder(); Set> allClasses = allClassesFinder.findClasses( makePackageName("", counter), false); assertEquals(3, allClasses.size()); @@ -114,7 +100,7 @@ public class TestClassFinder { packageAndLoadJar(c1, c2); packageAndLoadJar(c1); - ClassFinder allClassesFinder = new ClassFinder(trueNameFilter, trueClassFilter); + ClassFinder allClassesFinder = new ClassFinder(); Set> allClasses = allClassesFinder.findClasses( makePackageName("", counter), false); assertEquals(2, allClasses.size()); @@ -132,7 +118,7 @@ public class TestClassFinder { packageAndLoadJar(c1, c2); packageAndLoadJar(c3); - ClassFinder allClassesFinder = new ClassFinder(trueNameFilter, trueClassFilter); + ClassFinder allClassesFinder = new ClassFinder(); Set> nestedClasses = allClassesFinder.findClasses( makePackageName(NESTED, counter), false); assertEquals(2, nestedClasses.size()); @@ -158,7 +144,7 @@ public class TestClassFinder { return !fileName.startsWith(CLASSNAMEEXCPREFIX); } }; - ClassFinder incClassesFinder = new ClassFinder(notExcNameFilter, trueClassFilter); + ClassFinder incClassesFinder = new ClassFinder(null, notExcNameFilter, null); Set> incClasses = incClassesFinder.findClasses( makePackageName("", counter), false); assertEquals(1, incClasses.size()); @@ -182,7 +168,31 @@ public class TestClassFinder { return !c.getSimpleName().startsWith(CLASSNAMEEXCPREFIX); } }; - ClassFinder incClassesFinder = new ClassFinder(trueNameFilter, notExcClassFilter); + ClassFinder incClassesFinder = new ClassFinder(null, null, notExcClassFilter); + Set> incClasses = incClassesFinder.findClasses( + makePackageName("", counter), false); + assertEquals(1, incClasses.size()); + Class incClass = makeClass("", CLASSNAME, counter); + assertTrue(incClasses.contains(incClass)); + } + + @Test + public void testClassFinderFiltersByPathInJar() throws Exception { + final String CLASSNAME = "c1"; + long counter = testCounter.incrementAndGet(); + FileAndPath c1 = compileTestClass(counter, "", CLASSNAME); + FileAndPath c2 = compileTestClass(counter, "", "c2"); + packageAndLoadJar(c1); + final String excludedJar = packageAndLoadJar(c2); + + final ClassFinder.ResourcePathFilter notExcJarFilter = + new ClassFinder.ResourcePathFilter() { + @Override + public boolean isCandidatePath(String resourcePath, boolean isJar) { + return !isJar || !resourcePath.equals(excludedJar); + } + }; + ClassFinder incClassesFinder = new ClassFinder(notExcJarFilter, null, null); Set> incClasses = incClassesFinder.findClasses( makePackageName("", counter), false); assertEquals(1, incClasses.size()); @@ -194,7 +204,7 @@ public class TestClassFinder { public void testClassFinderCanFindClassesInDirs() throws Exception { // Well, technically, we are not guaranteed that the classes will // be in dirs, but during normal build they would be. - ClassFinder allClassesFinder = new ClassFinder(trueNameFilter, trueClassFilter); + ClassFinder allClassesFinder = new ClassFinder(); Set> allClasses = allClassesFinder.findClasses( this.getClass().getPackage().getName(), false); assertTrue(allClasses.contains(this.getClass())); @@ -204,16 +214,16 @@ public class TestClassFinder { @Test public void testClassFinderFiltersByNameInDirs() throws Exception { final String thisName = this.getClass().getSimpleName(); - ClassFinder.FileNameFilter notThisFilter = new ClassFinder.FileNameFilter() { + final ClassFinder.FileNameFilter notThisFilter = new ClassFinder.FileNameFilter() { @Override public boolean isCandidateFile(String fileName, String absFilePath) { return !fileName.equals(thisName + ".class"); } }; String thisPackage = this.getClass().getPackage().getName(); - ClassFinder allClassesFinder = new ClassFinder(trueNameFilter, trueClassFilter); + ClassFinder allClassesFinder = new ClassFinder(); Set> allClasses = allClassesFinder.findClasses(thisPackage, false); - ClassFinder notThisClassFinder = new ClassFinder(notThisFilter, trueClassFilter); + ClassFinder notThisClassFinder = new ClassFinder(null, notThisFilter, null); Set> notAllClasses = notThisClassFinder.findClasses(thisPackage, false); assertFalse(notAllClasses.contains(this.getClass())); assertEquals(allClasses.size() - 1, notAllClasses.size()); @@ -221,26 +231,42 @@ public class TestClassFinder { @Test public void testClassFinderFiltersByClassInDirs() throws Exception { - ClassFinder.ClassFilter notThisFilter = new ClassFinder.ClassFilter() { + final ClassFinder.ClassFilter notThisFilter = new ClassFinder.ClassFilter() { @Override public boolean isCandidateClass(Class c) { return c != TestClassFinder.class; } }; String thisPackage = this.getClass().getPackage().getName(); - ClassFinder allClassesFinder = new ClassFinder(trueNameFilter, trueClassFilter); + ClassFinder allClassesFinder = new ClassFinder(); Set> allClasses = allClassesFinder.findClasses(thisPackage, false); - ClassFinder notThisClassFinder = new ClassFinder(trueNameFilter, notThisFilter); + ClassFinder notThisClassFinder = new ClassFinder(null, null, notThisFilter); Set> notAllClasses = notThisClassFinder.findClasses(thisPackage, false); assertFalse(notAllClasses.contains(this.getClass())); assertEquals(allClasses.size() - 1, notAllClasses.size()); } + @Test + public void testClassFinderFiltersByPathInDirs() throws Exception { + final String hardcodedThisSubdir = "hbase-common"; + final ClassFinder.ResourcePathFilter notExcJarFilter = + new ClassFinder.ResourcePathFilter() { + @Override + public boolean isCandidatePath(String resourcePath, boolean isJar) { + return isJar || !resourcePath.contains(hardcodedThisSubdir); + } + }; + String thisPackage = this.getClass().getPackage().getName(); + ClassFinder notThisClassFinder = new ClassFinder(notExcJarFilter, null, null); + Set> notAllClasses = notThisClassFinder.findClasses(thisPackage, false); + assertFalse(notAllClasses.contains(this.getClass())); + } + @Test public void testClassFinderDefaultsToOwnPackage() throws Exception { // Correct handling of nested packages is tested elsewhere, so here we just assume // pkgClasses is the correct answer that we don't have to check. - ClassFinder allClassesFinder = new ClassFinder(trueNameFilter, trueClassFilter); + ClassFinder allClassesFinder = new ClassFinder(); Set> pkgClasses = allClassesFinder.findClasses( ClassFinder.class.getPackage().getName(), false); Set> defaultClasses = allClassesFinder.findClasses(false); @@ -295,8 +321,9 @@ public class TestClassFinder { /** * Makes a jar out of some class files. Unfortunately it's very tedious. * @param filesInJar Files created via compileTestClass. + * @return path to the resulting jar file. */ - private static void packageAndLoadJar(FileAndPath... filesInJar) throws Exception { + private static String packageAndLoadJar(FileAndPath... filesInJar) throws Exception { // First, write the bogus jar file. String path = basePath + "jar" + jarCounter.incrementAndGet() + ".jar"; Manifest manifest = new Manifest(); @@ -343,5 +370,6 @@ public class TestClassFinder { .getDeclaredMethod("addURL", new Class[] { URL.class }); method.setAccessible(true); method.invoke(urlClassLoader, new Object[] { jarFile.toURI().toURL() }); + return jarFile.getAbsolutePath(); } };