diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ClasspathScanner.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ClasspathScanner.java index eaad094ed79..975973a3f50 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ClasspathScanner.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ClasspathScanner.java @@ -15,7 +15,6 @@ import static org.junit.platform.commons.util.BlacklistedExceptions.rethrowIfBlacklisted; import java.io.File; -import java.io.IOException; import java.net.URL; import java.util.ArrayList; import java.util.Collections; @@ -25,6 +24,7 @@ import java.util.function.BiFunction; import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.logging.Level; import java.util.logging.Logger; import org.junit.platform.commons.meta.API; @@ -45,12 +45,18 @@ class ClasspathScanner { private static final String CLASS_FILE_SUFFIX = ".class"; + private static final String ROOT_PACKAGE_NAME = ""; + + /** Malformed class name InternalError like reported in #401. */ + private static final String MALFORMED_CLASS_NAME_ERROR_MESSAGE = "Malformed class name"; + private final Supplier classLoaderSupplier; private final BiFunction>> loadClass; ClasspathScanner(Supplier classLoaderSupplier, BiFunction>> loadClass) { + this.classLoaderSupplier = classLoaderSupplier; this.loadClass = loadClass; } @@ -58,45 +64,42 @@ class ClasspathScanner { boolean isPackage(String packageName) { Preconditions.notBlank(packageName, "package name must not be null or blank"); - String path = packagePath(packageName); try { - Enumeration resource = classLoaderSupplier.get().getResources(path); - return resource.hasMoreElements(); + return getClassLoader().getResources(packagePath(packageName)).hasMoreElements(); } - catch (IOException e) { + catch (Exception ex) { return false; } } List> scanForClassesInPackage(String basePackageName, Predicate> classFilter) { - Preconditions.notBlank(basePackageName, "basePackageName must not be blank"); + Preconditions.notBlank(basePackageName, "basePackageName must not be null or blank"); List dirs = allSourceDirsForPackage(basePackageName); return allClassesInSourceDirs(dirs, basePackageName, classFilter); } - private List> allClassesInSourceDirs(List sourceDirs, String basePackageName, - Predicate> classFilter) { - List> classes = new ArrayList<>(); - for (File aSourceDir : sourceDirs) { - classes.addAll(findClassesInSourceDirRecursively(aSourceDir, basePackageName, classFilter)); - } - return classes; - } - List> scanForClassesInClasspathRoot(File root, Predicate> classFilter) { Preconditions.notNull(root, "root must not be null"); Preconditions.condition(root.isDirectory(), () -> "root must be an existing directory: " + root.getAbsolutePath()); - return findClassesInSourceDirRecursively(root, "", classFilter); + return findClassesInSourceDirRecursively(ROOT_PACKAGE_NAME, root, classFilter); + } + + private List> allClassesInSourceDirs(List sourceDirs, String basePackageName, + Predicate> classFilter) { + + List> classes = new ArrayList<>(); + for (File sourceDir : sourceDirs) { + classes.addAll(findClassesInSourceDirRecursively(basePackageName, sourceDir, classFilter)); + } + return classes; } private List allSourceDirsForPackage(String basePackageName) { try { - ClassLoader classLoader = classLoaderSupplier.get(); - String path = packagePath(basePackageName); - Enumeration resources = classLoader.getResources(path); + Enumeration resources = getClassLoader().getResources(packagePath(basePackageName)); List dirs = new ArrayList<>(); while (resources.hasMoreElements()) { URL resource = resources.nextElement(); @@ -104,108 +107,120 @@ private List allSourceDirsForPackage(String basePackageName) { } return dirs; } - catch (IOException e) { + catch (Exception ex) { return Collections.emptyList(); } } - private String packagePath(String basePackageName) { - return basePackageName.replace('.', '/'); - } - - private List> findClassesInSourceDirRecursively(File sourceDir, String packageName, + private List> findClassesInSourceDirRecursively(String packageName, File sourceDir, Predicate> classFilter) { + Preconditions.notNull(classFilter, "classFilter must not be null"); - List> classesCollector = new ArrayList<>(); - collectClassesRecursively(sourceDir, packageName, classesCollector, classFilter); - return classesCollector; + List> classes = new ArrayList<>(); + collectClassesRecursively(packageName, sourceDir, classFilter, classes); + return classes; } - private void collectClassesRecursively(File sourceDir, String packageName, List> classesCollector, - Predicate> classFilter) { + private void collectClassesRecursively(String packageName, File sourceDir, Predicate> classFilter, + List> classes) { + File[] files = sourceDir.listFiles(); if (files == null) { return; } + for (File file : files) { if (isClassFile(file)) { - this.handleClassFileSafely(packageName, classesCollector, classFilter, file); + processClassFileSafely(packageName, file, classFilter, classes); } else if (file.isDirectory()) { - collectClassesRecursively(file, appendPackageName(packageName, file.getName()), classesCollector, - classFilter); + collectClassesRecursively(appendSubpackageName(packageName, file.getName()), file, classFilter, + classes); } } } - private void handleClassFileSafely(String packageName, List> classesCollector, - Predicate> classFilter, File file) { - Optional> classForClassFile = Optional.empty(); + private void processClassFileSafely(String packageName, File classFile, Predicate> classFilter, + List> classes) { + Optional> clazz = Optional.empty(); try { - classForClassFile = loadClassForClassFile(file, packageName); - classForClassFile.filter(classFilter).ifPresent(classesCollector::add); + clazz = loadClassFromFile(packageName, classFile); + clazz.filter(classFilter).ifPresent(classes::add); } catch (InternalError internalError) { - this.catchInternalError(file, classForClassFile, internalError); + handleInternalError(classFile, clazz, internalError); } catch (Throwable throwable) { - this.catchAllOtherThrowables(file, throwable); + handleThrowable(classFile, throwable); } } - private void catchInternalError(File file, Optional> classForClassFile, InternalError internalError) { - // Malformed class name InternalError as reported by #401 - if (internalError.getMessage().equals("Malformed class name")) { - this.logMalformedClassnameInternalError(file, classForClassFile); + private Optional> loadClassFromFile(String packageName, File classFile) { + String className = packageName + '.' + + classFile.getName().substring(0, classFile.getName().length() - CLASS_FILE_SUFFIX.length()); + return this.loadClass.apply(className, getClassLoader()); + } + + private void handleInternalError(File classFile, Optional> clazz, InternalError ex) { + if (MALFORMED_CLASS_NAME_ERROR_MESSAGE.equals(ex.getMessage())) { + logMalformedClassName(classFile, clazz, ex); } - // other potential InternalErrors else { - this.logGenericFileProcessingProblem(file); + logGenericFileProcessingException(classFile, ex); } } - private void logMalformedClassnameInternalError(File file, Optional> classForClassFile) { + private void handleThrowable(File classFile, Throwable throwable) { + rethrowIfBlacklisted(throwable); + logGenericFileProcessingException(classFile, throwable); + } + + private void logMalformedClassName(File classFile, Optional> clazz, InternalError ex) { try { - LOG.warning(() -> format("The class in the current file has a malformed class name. Offending file: '%s'", - file.getAbsolutePath())); - classForClassFile.ifPresent(malformedClass -> - //cannot use getCanonicalName() here because its being null is related to the underlying error - LOG.warning(() -> format("Malformed class name: '%s'", malformedClass.getName()))); + if (clazz.isPresent()) { + // Do not use getSimpleName() or getCanonicalName() here because they will likely + // throw another exception due to the underlying error. + logWarning(ex, + () -> format("The java.lang.Class loaded from file [%s] has a malformed class name [%s].", + classFile.getAbsolutePath(), clazz.get().getName())); + } + else { + logWarning(ex, () -> format("The java.lang.Class loaded from file [%s] has a malformed class name.", + classFile.getAbsolutePath())); + } } - catch (Throwable throwable) { - LOG.warning( - "The class name of the class in the current file is so malformed that not even getName() or toString() can be called on it!"); + catch (Throwable t) { + ex.addSuppressed(t); + logGenericFileProcessingException(classFile, ex); } } - private void catchAllOtherThrowables(File file, Throwable throwable) { - rethrowIfBlacklisted(throwable); - this.logGenericFileProcessingProblem(file); - } - - private void logGenericFileProcessingProblem(File file) { - LOG.warning(() -> format("There was a problem while processing the current file. Offending file: '%s'", - file.getAbsolutePath())); + private void logGenericFileProcessingException(File classFile, Throwable throwable) { + logWarning(throwable, () -> format("Failed to load java.lang.Class for file [%s] during classpath scanning.", + classFile.getAbsolutePath())); } - private String appendPackageName(String packageName, String subpackageName) { - if (packageName.isEmpty()) - return subpackageName; - else - return packageName + "." + subpackageName; + private String appendSubpackageName(String packageName, String subpackageName) { + return (!packageName.isEmpty() ? packageName + "." + subpackageName : subpackageName); } - private Optional> loadClassForClassFile(File file, String packageName) { - String className = packageName + '.' - + file.getName().substring(0, file.getName().length() - CLASS_FILE_SUFFIX.length()); - return loadClass.apply(className, classLoaderSupplier.get()); + private ClassLoader getClassLoader() { + return this.classLoaderSupplier.get(); } private static boolean isClassFile(File file) { return file.isFile() && file.getName().endsWith(CLASS_FILE_SUFFIX); } + private static String packagePath(String packageName) { + return packageName.replace('.', '/'); + } + + private static void logWarning(Throwable throwable, Supplier msgSupplier) { + LOG.log(Level.WARNING, throwable, msgSupplier); + } + } diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/ClasspathScannerTests.java b/platform-tests/src/test/java/org/junit/platform/commons/util/ClasspathScannerTests.java index 2299d2c7ed7..8cb6abe873a 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/util/ClasspathScannerTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/ClasspathScannerTests.java @@ -22,7 +22,6 @@ import java.util.Enumeration; import java.util.List; import java.util.function.Predicate; -import java.util.function.Supplier; import org.junit.jupiter.api.Test; @@ -36,68 +35,73 @@ class ClasspathScannerTests { private final ClasspathScanner classpathScanner = new ClasspathScanner(ReflectionUtils::getDefaultClassLoader, ReflectionUtils::loadClass); + @Test + void scanForClassesInClasspathRootWhenMalformedClassnameInternalErrorOccursWithNullDetailedMessage() + throws Exception { + + Predicate> malformedClassNameSimulationFilter = clazz -> { + if (clazz.getSimpleName().equals(ClassForMalformedClassNameSimulation.class.getSimpleName())) { + throw new InternalError(); + } + return true; + }; + + assertClassesScannedWhenExceptionIsThrown(malformedClassNameSimulationFilter); + } + @Test void scanForClassesInClasspathRootWhenMalformedClassnameInternalErrorOccurs() throws Exception { Predicate> malformedClassNameSimulationFilter = clazz -> { - if (clazz.getSimpleName().equals("ClassForMalformedClassNameSimulation")) + if (clazz.getSimpleName().equals(ClassForMalformedClassNameSimulation.class.getSimpleName())) { throw new InternalError("Malformed class name"); - + } return true; }; - File root = getTestClasspathRoot(); - List> classes = this.classpathScanner.scanForClassesInClasspathRoot(root, - malformedClassNameSimulationFilter); - - assertThat(classes.size()).isGreaterThanOrEqualTo(150); + assertClassesScannedWhenExceptionIsThrown(malformedClassNameSimulationFilter); } @Test void scanForClassesInClasspathRootWhenOtherInternalErrorOccurs() throws Exception { Predicate> otherInternalErrorSimulationFilter = clazz -> { - if (clazz.getSimpleName().equals("ClassForOtherInternalErrorSimulation")) + if (clazz.getSimpleName().equals(ClassForOtherInternalErrorSimulation.class.getSimpleName())) { throw new InternalError("other internal error"); - + } return true; }; - File root = getTestClasspathRoot(); - List> classes = this.classpathScanner.scanForClassesInClasspathRoot(root, - otherInternalErrorSimulationFilter); - - assertThat(classes.size()).isGreaterThanOrEqualTo(150); + assertClassesScannedWhenExceptionIsThrown(otherInternalErrorSimulationFilter); } @Test void scanForClassesInClasspathRootWhenGenericRuntimeExceptionOccurs() throws Exception { Predicate> runtimeExceptionSimulationFilter = clazz -> { - if (clazz.getSimpleName().equals("ClassForGenericRuntimeExceptionSimulation")) + if (clazz.getSimpleName().equals(ClassForGenericRuntimeExceptionSimulation.class.getSimpleName())) { throw new RuntimeException("a generic exception"); - + } return true; }; - File root = getTestClasspathRoot(); - List> classes = this.classpathScanner.scanForClassesInClasspathRoot(root, - runtimeExceptionSimulationFilter); + assertClassesScannedWhenExceptionIsThrown(runtimeExceptionSimulationFilter); + } + private void assertClassesScannedWhenExceptionIsThrown(Predicate> filter) throws Exception { + List> classes = this.classpathScanner.scanForClassesInClasspathRoot(getTestClasspathRoot(), filter); assertThat(classes.size()).isGreaterThanOrEqualTo(150); } @Test void scanForClassesInClasspathRootWhenOutOfMemoryErrorOccurs() throws Exception { Predicate> outOfMemoryErrorSimulationFilter = clazz -> { - if (clazz.getSimpleName().equals("ClassForOutOfMemoryErrorSimulation")) + if (clazz.getSimpleName().equals(ClassForOutOfMemoryErrorSimulation.class.getSimpleName())) { throw new OutOfMemoryError(); - + } return true; }; - File root = getTestClasspathRoot(); - assertThrows(OutOfMemoryError.class, () -> { - this.classpathScanner.scanForClassesInClasspathRoot(root, outOfMemoryErrorSimulationFilter); - }); - + assertThrows(OutOfMemoryError.class, + () -> this.classpathScanner.scanForClassesInClasspathRoot(getTestClasspathRoot(), + outOfMemoryErrorSimulationFilter)); } @Test @@ -129,7 +133,7 @@ void scanForClassesInPackageForNullClassFilter() { @Test void scanForClassesInPackageWhenIOExceptionOccurs() { - ClasspathScanner scanner = new ClasspathScanner(new ThrowingClassLoaderSupplier(), ReflectionUtils::loadClass); + ClasspathScanner scanner = new ClasspathScanner(() -> new ThrowingClassLoader(), ReflectionUtils::loadClass); List> classes = scanner.scanForClassesInPackage("org.junit.platform.commons", clazz -> true); assertThat(classes).isEmpty(); } @@ -147,7 +151,7 @@ void isPackageForNullPackageName() { @Test void isPackageWhenIOExceptionOccurs() { - ClasspathScanner scanner = new ClasspathScanner(new ThrowingClassLoaderSupplier(), ReflectionUtils::loadClass); + ClasspathScanner scanner = new ClasspathScanner(() -> new ThrowingClassLoader(), ReflectionUtils::loadClass); assertFalse(scanner.isPackage("org.junit.platform.commons")); } @@ -209,14 +213,6 @@ static class ClassForGenericRuntimeExceptionSimulation { static class ClassForOutOfMemoryErrorSimulation { } - private static class ThrowingClassLoaderSupplier implements Supplier { - - @Override - public ClassLoader get() { - return new ThrowingClassLoader(); - } - } - private static class ThrowingClassLoader extends ClassLoader { @Override diff --git a/platform-tests/src/test/resources/log4j2-test.xml b/platform-tests/src/test/resources/log4j2-test.xml index ad72dd6c70d..4c9685185b6 100644 --- a/platform-tests/src/test/resources/log4j2-test.xml +++ b/platform-tests/src/test/resources/log4j2-test.xml @@ -7,6 +7,7 @@ +