Skip to content

Commit

Permalink
Use Path instead of File in ClasspathScanner
Browse files Browse the repository at this point in the history 
Issue: #466
  • Loading branch information
@marcphilipp
marcphilipp committed Aug 27, 2016
1 parent d268d9f commit a498e23
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 57 deletions.
3 changes: 2 additions & 1 deletion ...er-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java
View file
Expand Up @@ -14,6 +14,7 @@
import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInClasspathRoot; import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInClasspathRoot;
import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInPackage; import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInPackage;


import java.nio.file.Paths;
import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;


Expand Down Expand Up @@ -45,7 +46,7 @@ public void resolveSelectors(EngineDiscoveryRequest request, TestDescriptor engi
JavaElementsResolver javaElementsResolver = createJavaElementsResolver(engineDescriptor); JavaElementsResolver javaElementsResolver = createJavaElementsResolver(engineDescriptor);


request.getSelectorsByType(ClasspathRootSelector.class).forEach(selector -> { request.getSelectorsByType(ClasspathRootSelector.class).forEach(selector -> {
findAllClassesInClasspathRoot(selector.getClasspathRoot(), isScannableTestClass).forEach( findAllClassesInClasspathRoot(Paths.get(selector.getClasspathRoot()), isScannableTestClass).forEach(
javaElementsResolver::resolveClass); javaElementsResolver::resolveClass);
}); });
request.getSelectorsByType(JavaPackageSelector.class).forEach(selector -> { request.getSelectorsByType(JavaPackageSelector.class).forEach(selector -> {
Expand Down
88 changes: 49 additions & 39 deletions junit-platform-commons/src/main/java/org/junit/platform/commons/util/ClasspathScanner.java
View file
Expand Up @@ -15,8 +15,11 @@
import static org.junit.platform.commons.meta.API.Usage.Internal; import static org.junit.platform.commons.meta.API.Usage.Internal;
import static org.junit.platform.commons.util.BlacklistedExceptions.rethrowIfBlacklisted; import static org.junit.platform.commons.util.BlacklistedExceptions.rethrowIfBlacklisted;


import java.io.File; import java.io.IOException;
import java.net.URL; import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
Expand Down Expand Up @@ -83,10 +86,10 @@ List<Class<?>> scanForClassesInPackage(String basePackageName, Predicate<Class<?
return findClassesInSourceDirs(getSourceDirsForPackage(basePackageName), basePackageName, classFilter); return findClassesInSourceDirs(getSourceDirsForPackage(basePackageName), basePackageName, classFilter);
} }


List<Class<?>> scanForClassesInClasspathRoot(File root, Predicate<Class<?>> classFilter) { List<Class<?>> scanForClassesInClasspathRoot(Path root, Predicate<Class<?>> classFilter) {
Preconditions.notNull(root, "root must not be null"); Preconditions.notNull(root, "root must not be null");
Preconditions.condition(root.isDirectory(), Preconditions.condition(Files.isDirectory(root),
() -> "root must be an existing directory: " + root.getAbsolutePath()); () -> "root must be an existing directory: " + root.toAbsolutePath());
Preconditions.notNull(classFilter, "classFilter must not be null"); Preconditions.notNull(classFilter, "classFilter must not be null");


return findClassesInSourceDir(DEFAULT_PACKAGE_NAME, root, classFilter); return findClassesInSourceDir(DEFAULT_PACKAGE_NAME, root, classFilter);
Expand All @@ -95,7 +98,7 @@ List<Class<?>> scanForClassesInClasspathRoot(File root, Predicate<Class<?>> clas
/** /**
* Recursively scan for classes in all of the supplied source directories. * Recursively scan for classes in all of the supplied source directories.
*/ */
private List<Class<?>> findClassesInSourceDirs(List<File> sourceDirs, String basePackageName, private List<Class<?>> findClassesInSourceDirs(List<Path> sourceDirs, String basePackageName,
Predicate<Class<?>> classFilter) { Predicate<Class<?>> classFilter) {


// @formatter:off // @formatter:off
Expand All @@ -110,31 +113,31 @@ private List<Class<?>> findClassesInSourceDirs(List<File> sourceDirs, String bas
/** /**
* Recursively scan for classes in the supplied source directory. * Recursively scan for classes in the supplied source directory.
*/ */
private List<Class<?>> findClassesInSourceDir(String packageName, File sourceDir, Predicate<Class<?>> classFilter) { private List<Class<?>> findClassesInSourceDir(String packageName, Path sourceDir, Predicate<Class<?>> classFilter) {
List<Class<?>> classes = new ArrayList<>(); List<Class<?>> classes = new ArrayList<>();
findClassesInSourceDir(packageName, sourceDir, classFilter, classes); findClassesInSourceDir(packageName, sourceDir, classFilter, classes);
return classes; return classes;
} }


private void findClassesInSourceDir(String packageName, File sourceDir, Predicate<Class<?>> classFilter, private void findClassesInSourceDir(String packageName, Path sourceDir, Predicate<Class<?>> classFilter,
List<Class<?>> classes) { List<Class<?>> classes) {

try {
File[] files = sourceDir.listFiles(); Files.list(sourceDir).forEach(path -> {
if (files == null) { if (isNotPackageInfo(path) && isClassFile(path)) {
return; processClassFileSafely(packageName, path, classFilter, classes);
}
else if (Files.isDirectory(path)) {
findClassesInSourceDir(appendSubpackageName(packageName, path.getFileName().toString()), path,
classFilter, classes);
}
});
} }

catch (IOException ex) {
for (File file : files) { logWarning(ex, () -> "I/O Error reading files in " + sourceDir);
if (isNotPackageInfo(file) && isClassFile(file)) {
processClassFileSafely(packageName, file, classFilter, classes);
}
else if (file.isDirectory()) {
findClassesInSourceDir(appendSubpackageName(packageName, file.getName()), file, classFilter, classes);
}
} }
} }


private void processClassFileSafely(String packageName, File classFile, Predicate<Class<?>> classFilter, private void processClassFileSafely(String packageName, Path classFile, Predicate<Class<?>> classFilter,
List<Class<?>> classes) { List<Class<?>> classes) {


Optional<Class<?>> clazz = Optional.empty(); Optional<Class<?>> clazz = Optional.empty();
Expand All @@ -150,16 +153,17 @@ private void processClassFileSafely(String packageName, File classFile, Predicat
} }
} }


private Optional<Class<?>> loadClassFromFile(String packageName, File classFile) { private Optional<Class<?>> loadClassFromFile(String packageName, Path classFile) {
String className = classFile.getName().substring(0, classFile.getName().length() - CLASS_FILE_SUFFIX.length()); String fileName = classFile.getFileName().toString();
String className = fileName.substring(0, fileName.length() - CLASS_FILE_SUFFIX.length());


// Handle default package appropriately. // Handle default package appropriately.
String fqcn = StringUtils.isBlank(packageName) ? className : packageName + "." + className; String fqcn = StringUtils.isBlank(packageName) ? className : packageName + "." + className;


return this.loadClass.apply(fqcn, getClassLoader()); return this.loadClass.apply(fqcn, getClassLoader());
} }


private void handleInternalError(File classFile, Optional<Class<?>> clazz, InternalError ex) { private void handleInternalError(Path classFile, Optional<Class<?>> clazz, InternalError ex) {
if (MALFORMED_CLASS_NAME_ERROR_MESSAGE.equals(ex.getMessage())) { if (MALFORMED_CLASS_NAME_ERROR_MESSAGE.equals(ex.getMessage())) {
logMalformedClassName(classFile, clazz, ex); logMalformedClassName(classFile, clazz, ex);
} }
Expand All @@ -168,24 +172,24 @@ private void handleInternalError(File classFile, Optional<Class<?>> clazz, Inter
} }
} }


private void handleThrowable(File classFile, Throwable throwable) { private void handleThrowable(Path classFile, Throwable throwable) {
rethrowIfBlacklisted(throwable); rethrowIfBlacklisted(throwable);
logGenericFileProcessingException(classFile, throwable); logGenericFileProcessingException(classFile, throwable);
} }


private void logMalformedClassName(File classFile, Optional<Class<?>> clazz, InternalError ex) { private void logMalformedClassName(Path classFile, Optional<Class<?>> clazz, InternalError ex) {
try { try {


if (clazz.isPresent()) { if (clazz.isPresent()) {
// Do not use getSimpleName() or getCanonicalName() here because they will likely // Do not use getSimpleName() or getCanonicalName() here because they will likely
// throw another exception due to the underlying error. // throw another exception due to the underlying error.
logWarning(ex, logWarning(ex,
() -> format("The java.lang.Class loaded from file [%s] has a malformed class name [%s].", () -> format("The java.lang.Class loaded from path [%s] has a malformed class name [%s].",
classFile.getAbsolutePath(), clazz.get().getName())); classFile.toAbsolutePath(), clazz.get().getName()));
} }
else { else {
logWarning(ex, () -> format("The java.lang.Class loaded from file [%s] has a malformed class name.", logWarning(ex, () -> format("The java.lang.Class loaded from path [%s] has a malformed class name.",
classFile.getAbsolutePath())); classFile.toAbsolutePath()));
} }
} }
catch (Throwable t) { catch (Throwable t) {
Expand All @@ -194,9 +198,9 @@ private void logMalformedClassName(File classFile, Optional<Class<?>> clazz, Int
} }
} }


private void logGenericFileProcessingException(File classFile, Throwable throwable) { private void logGenericFileProcessingException(Path classFile, Throwable throwable) {
logWarning(throwable, () -> format("Failed to load java.lang.Class for file [%s] during classpath scanning.", logWarning(throwable, () -> format("Failed to load java.lang.Class for path [%s] during classpath scanning.",
classFile.getAbsolutePath())); classFile.toAbsolutePath()));
} }


private String appendSubpackageName(String packageName, String subpackageName) { private String appendSubpackageName(String packageName, String subpackageName) {
Expand All @@ -213,29 +217,35 @@ private static void assertPackageNameIsPlausible(String packageName) {
"package name must not contain only whitespace"); "package name must not contain only whitespace");
} }


private static boolean isNotPackageInfo(File file) { private static boolean isNotPackageInfo(Path path) {
return !"package-info.class".equals(file.getName()); return !path.endsWith("package-info.class");
} }


private static boolean isClassFile(File file) { private static boolean isClassFile(Path path) {
return file.isFile() && file.getName().endsWith(CLASS_FILE_SUFFIX); return Files.isRegularFile(path) && path.getFileName().toString().endsWith(CLASS_FILE_SUFFIX);
} }


private static String packagePath(String packageName) { private static String packagePath(String packageName) {
return packageName.replace('.', '/'); return packageName.replace('.', '/');
} }


private List<File> getSourceDirsForPackage(String packageName) { private List<Path> getSourceDirsForPackage(String packageName) {
try { try {
Enumeration<URL> resources = getClassLoader().getResources(packagePath(packageName)); Enumeration<URL> resources = getClassLoader().getResources(packagePath(packageName));
List<File> dirs = new ArrayList<>(); List<Path> dirs = new ArrayList<>();
while (resources.hasMoreElements()) { while (resources.hasMoreElements()) {
URL resource = resources.nextElement(); URL resource = resources.nextElement();
dirs.add(new File(resource.getFile())); try {
dirs.add(Paths.get(resource.toURI()));
}
catch (Exception ex) {
logWarning(ex, () -> "Error converting directory to Path for " + resource);
}
} }
return dirs; return dirs;
} }
catch (Exception ex) { catch (Exception ex) {
logWarning(ex, () -> "Error reading directories from class loader for package " + packageName);
return Collections.emptyList(); return Collections.emptyList();
} }
} }
Expand Down
3 changes: 2 additions & 1 deletion junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java
View file
Expand Up @@ -23,6 +23,7 @@
import java.lang.reflect.Member; import java.lang.reflect.Member;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.lang.reflect.Modifier; import java.lang.reflect.Modifier;
import java.nio.file.Path;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
Expand Down Expand Up @@ -448,7 +449,7 @@ public static Set<File> getAllClasspathRootDirectories() {
// @formatter:on // @formatter:on
} }


public static List<Class<?>> findAllClassesInClasspathRoot(File root, Predicate<Class<?>> classTester) { public static List<Class<?>> findAllClassesInClasspathRoot(Path root, Predicate<Class<?>> classTester) {
return classpathScanner.scanForClassesInClasspathRoot(root, classTester); return classpathScanner.scanForClassesInClasspathRoot(root, classTester);
} }


Expand Down
10 changes: 5 additions & 5 deletions ...tform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathRootSelector.java
View file
Expand Up @@ -12,7 +12,7 @@


import static org.junit.platform.commons.meta.API.Usage.Experimental; import static org.junit.platform.commons.meta.API.Usage.Experimental;


import java.io.File; import java.net.URI;


import org.junit.platform.commons.meta.API; import org.junit.platform.commons.meta.API;
import org.junit.platform.commons.util.ToStringBuilder; import org.junit.platform.commons.util.ToStringBuilder;
Expand All @@ -30,16 +30,16 @@
@API(Experimental) @API(Experimental)
public class ClasspathRootSelector implements DiscoverySelector { public class ClasspathRootSelector implements DiscoverySelector {


private final File classpathRoot; private final URI classpathRoot;


ClasspathRootSelector(File classpathRoot) { ClasspathRootSelector(URI classpathRoot) {
this.classpathRoot = classpathRoot; this.classpathRoot = classpathRoot;
} }


/** /**
* Get the selected classpath root directory. * Get the selected classpath root directory as an {@link URI}.
*/ */
public File getClasspathRoot() { public URI getClasspathRoot() {
return this.classpathRoot; return this.classpathRoot;
} }


Expand Down
1 change: 1 addition & 0 deletions ...platform-engine/src/main/java/org/junit/platform/engine/discovery/DiscoverySelectors.java
View file
Expand Up @@ -188,6 +188,7 @@ public static List<ClasspathRootSelector> selectClasspathRoots(Set<File> directo
// @formatter:off // @formatter:off
return directories.stream() return directories.stream()
.filter(File::isDirectory) .filter(File::isDirectory)
.map(File::toURI)
.map(ClasspathRootSelector::new) .map(ClasspathRootSelector::new)
.collect(toList()); .collect(toList());
// @formatter:on // @formatter:on
Expand Down
5 changes: 4 additions & 1 deletion ...ngine/src/main/java/org/junit/vintage/engine/discovery/ClasspathRootSelectorResolver.java
View file
Expand Up @@ -12,6 +12,8 @@


import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInClasspathRoot; import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInClasspathRoot;


import java.nio.file.Paths;

import org.junit.platform.engine.discovery.ClasspathRootSelector; import org.junit.platform.engine.discovery.ClasspathRootSelector;


/** /**
Expand All @@ -25,7 +27,8 @@ class ClasspathRootSelectorResolver extends DiscoverySelectorResolver<ClasspathR


@Override @Override
void resolve(ClasspathRootSelector selector, TestClassCollector collector) { void resolve(ClasspathRootSelector selector, TestClassCollector collector) {
findAllClassesInClasspathRoot(selector.getClasspathRoot(), classTester).forEach(collector::addCompletely); findAllClassesInClasspathRoot(Paths.get(selector.getClasspathRoot()), classTester).forEach(
collector::addCompletely);
} }


} }
13 changes: 7 additions & 6 deletions platform-tests/src/test/java/org/junit/platform/commons/util/ClasspathScannerTests.java
View file
Expand Up @@ -17,9 +17,10 @@
import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;


import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.net.URL; import java.net.URL;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Enumeration; import java.util.Enumeration;
import java.util.List; import java.util.List;
import java.util.function.Predicate; import java.util.function.Predicate;
Expand Down Expand Up @@ -181,7 +182,7 @@ void isPackageWhenIOExceptionOccurs() {
@Test @Test
void findAllClassesInClasspathRoot() throws Exception { void findAllClassesInClasspathRoot() throws Exception {
Predicate<Class<?>> thisClassOnly = clazz -> clazz == ClasspathScannerTests.class; Predicate<Class<?>> thisClassOnly = clazz -> clazz == ClasspathScannerTests.class;
File root = getTestClasspathRoot(); Path root = getTestClasspathRoot();
List<Class<?>> classes = classpathScanner.scanForClassesInClasspathRoot(root, thisClassOnly); List<Class<?>> classes = classpathScanner.scanForClassesInClasspathRoot(root, thisClassOnly);
assertSame(ClasspathScannerTests.class, classes.get(0)); assertSame(ClasspathScannerTests.class, classes.get(0));
} }
Expand All @@ -205,7 +206,7 @@ private boolean inDefaultPackage(Class<?> clazz) {


@Test @Test
void findAllClassesInClasspathRootWithFilter() throws Exception { void findAllClassesInClasspathRootWithFilter() throws Exception {
File root = getTestClasspathRoot(); Path root = getTestClasspathRoot();
List<Class<?>> classes = classpathScanner.scanForClassesInClasspathRoot(root, clazz -> true); List<Class<?>> classes = classpathScanner.scanForClassesInClasspathRoot(root, clazz -> true);


assertThat(classes.size()).isGreaterThanOrEqualTo(20); assertThat(classes.size()).isGreaterThanOrEqualTo(20);
Expand All @@ -221,7 +222,7 @@ void findAllClassesInClasspathRootForNullRoot() throws Exception {
@Test @Test
void findAllClassesInClasspathRootForNonExistingRoot() throws Exception { void findAllClassesInClasspathRootForNonExistingRoot() throws Exception {
assertThrows(PreconditionViolationException.class, assertThrows(PreconditionViolationException.class,
() -> classpathScanner.scanForClassesInClasspathRoot(new File("does_not_exist"), clazz -> true)); () -> classpathScanner.scanForClassesInClasspathRoot(Paths.get("does_not_exist"), clazz -> true));
} }


@Test @Test
Expand All @@ -230,9 +231,9 @@ void findAllClassesInClasspathRootForNullClassFilter() throws Exception {
() -> classpathScanner.scanForClassesInClasspathRoot(getTestClasspathRoot(), null)); () -> classpathScanner.scanForClassesInClasspathRoot(getTestClasspathRoot(), null));
} }


private File getTestClasspathRoot() throws Exception { private Path getTestClasspathRoot() throws Exception {
URL location = getClass().getProtectionDomain().getCodeSource().getLocation(); URL location = getClass().getProtectionDomain().getCodeSource().getLocation();
return new File(location.toURI()); return Paths.get(location.toURI());
} }


class MemberClassToBeFound { class MemberClassToBeFound {
Expand Down
7 changes: 4 additions & 3 deletions ...rm-tests/src/test/java/org/junit/platform/console/tasks/DiscoveryRequestCreatorTests.java
View file
Expand Up @@ -18,6 +18,7 @@


import java.io.File; import java.io.File;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.net.URI;
import java.util.List; import java.util.List;


import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -84,7 +85,7 @@ public void convertsAllOptionWithoutExplicitRootDirectories() {
List<ClasspathRootSelector> classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class); List<ClasspathRootSelector> classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class);
// @formatter:off // @formatter:off
assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot) assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot)
.hasAtLeastOneElementOfType(File.class) .hasAtLeastOneElementOfType(URI.class)
.doesNotContainNull(); .doesNotContainNull();
// @formatter:on // @formatter:on
} }
Expand All @@ -99,7 +100,7 @@ public void convertsAllOptionWithExplicitRootDirectories() {
List<ClasspathRootSelector> classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class); List<ClasspathRootSelector> classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class);
// @formatter:off // @formatter:off
assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot) assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot)
.containsExactly(new File("."), new File("..")); .containsExactly(new File(".").toURI(), new File("..").toURI());
// @formatter:on // @formatter:on
} }


Expand All @@ -113,7 +114,7 @@ public void convertsAllOptionWithAdditionalClasspathEntries() {
List<ClasspathRootSelector> classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class); List<ClasspathRootSelector> classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class);
// @formatter:off // @formatter:off
assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot) assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot)
.contains(new File("."), new File("..")); .contains(new File(".").toURI(), new File("..").toURI());
// @formatter:on // @formatter:on
} }


Expand Down
3 changes: 2 additions & 1 deletion .../src/test/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilderTests.java
View file
Expand Up @@ -166,7 +166,8 @@ public void availableFoldersAreStoredInDiscoveryRequest() throws Exception {
// @formatter:on // @formatter:on


List<String> folders = discoveryRequest.getSelectorsByType(ClasspathRootSelector.class).stream().map( List<String> folders = discoveryRequest.getSelectorsByType(ClasspathRootSelector.class).stream().map(
ClasspathRootSelector::getClasspathRoot).map(File::getAbsolutePath).collect(toList()); ClasspathRootSelector::getClasspathRoot).map(File::new).map(File::getAbsolutePath).collect(
toList());


assertThat(folders).contains(temporaryFolder.getAbsolutePath()); assertThat(folders).contains(temporaryFolder.getAbsolutePath());
} }
Expand Down

0 comments on commit a498e23

Please sign in to comment.