From a498e23bcbbeea5ff2e6c9f0234f968602d6df4e Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Thu, 25 Aug 2016 16:51:08 +0200 Subject: [PATCH] Use Path instead of File in ClasspathScanner Issue: #466 --- .../discovery/DiscoverySelectorResolver.java | 3 +- .../commons/util/ClasspathScanner.java | 88 +++++++++++-------- .../commons/util/ReflectionUtils.java | 3 +- .../discovery/ClasspathRootSelector.java | 10 +-- .../engine/discovery/DiscoverySelectors.java | 1 + .../ClasspathRootSelectorResolver.java | 5 +- .../commons/util/ClasspathScannerTests.java | 13 +-- .../tasks/DiscoveryRequestCreatorTests.java | 7 +- .../LauncherDiscoveryRequestBuilderTests.java | 3 +- 9 files changed, 76 insertions(+), 57 deletions(-) diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java index 49ed1d61236..5f82715f762 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java @@ -14,6 +14,7 @@ import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInClasspathRoot; import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInPackage; +import java.nio.file.Paths; import java.util.HashSet; import java.util.Set; @@ -45,7 +46,7 @@ public void resolveSelectors(EngineDiscoveryRequest request, TestDescriptor engi JavaElementsResolver javaElementsResolver = createJavaElementsResolver(engineDescriptor); request.getSelectorsByType(ClasspathRootSelector.class).forEach(selector -> { - findAllClassesInClasspathRoot(selector.getClasspathRoot(), isScannableTestClass).forEach( + findAllClassesInClasspathRoot(Paths.get(selector.getClasspathRoot()), isScannableTestClass).forEach( javaElementsResolver::resolveClass); }); request.getSelectorsByType(JavaPackageSelector.class).forEach(selector -> { 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 9474bee1d48..caec51b9c33 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,8 +15,11 @@ import static org.junit.platform.commons.meta.API.Usage.Internal; import static org.junit.platform.commons.util.BlacklistedExceptions.rethrowIfBlacklisted; -import java.io.File; +import java.io.IOException; 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.Collection; import java.util.Collections; @@ -83,10 +86,10 @@ List> scanForClassesInPackage(String basePackageName, Predicate> scanForClassesInClasspathRoot(File root, Predicate> classFilter) { + List> scanForClassesInClasspathRoot(Path root, Predicate> classFilter) { Preconditions.notNull(root, "root must not be null"); - Preconditions.condition(root.isDirectory(), - () -> "root must be an existing directory: " + root.getAbsolutePath()); + Preconditions.condition(Files.isDirectory(root), + () -> "root must be an existing directory: " + root.toAbsolutePath()); Preconditions.notNull(classFilter, "classFilter must not be null"); return findClassesInSourceDir(DEFAULT_PACKAGE_NAME, root, classFilter); @@ -95,7 +98,7 @@ List> scanForClassesInClasspathRoot(File root, Predicate> clas /** * Recursively scan for classes in all of the supplied source directories. */ - private List> findClassesInSourceDirs(List sourceDirs, String basePackageName, + private List> findClassesInSourceDirs(List sourceDirs, String basePackageName, Predicate> classFilter) { // @formatter:off @@ -110,31 +113,31 @@ private List> findClassesInSourceDirs(List sourceDirs, String bas /** * Recursively scan for classes in the supplied source directory. */ - private List> findClassesInSourceDir(String packageName, File sourceDir, Predicate> classFilter) { + private List> findClassesInSourceDir(String packageName, Path sourceDir, Predicate> classFilter) { List> classes = new ArrayList<>(); findClassesInSourceDir(packageName, sourceDir, classFilter, classes); return classes; } - private void findClassesInSourceDir(String packageName, File sourceDir, Predicate> classFilter, + private void findClassesInSourceDir(String packageName, Path sourceDir, Predicate> classFilter, List> classes) { - - File[] files = sourceDir.listFiles(); - if (files == null) { - return; + try { + Files.list(sourceDir).forEach(path -> { + if (isNotPackageInfo(path) && isClassFile(path)) { + processClassFileSafely(packageName, path, classFilter, classes); + } + else if (Files.isDirectory(path)) { + findClassesInSourceDir(appendSubpackageName(packageName, path.getFileName().toString()), path, + classFilter, classes); + } + }); } - - for (File file : files) { - if (isNotPackageInfo(file) && isClassFile(file)) { - processClassFileSafely(packageName, file, classFilter, classes); - } - else if (file.isDirectory()) { - findClassesInSourceDir(appendSubpackageName(packageName, file.getName()), file, classFilter, classes); - } + catch (IOException ex) { + logWarning(ex, () -> "I/O Error reading files in " + sourceDir); } } - private void processClassFileSafely(String packageName, File classFile, Predicate> classFilter, + private void processClassFileSafely(String packageName, Path classFile, Predicate> classFilter, List> classes) { Optional> clazz = Optional.empty(); @@ -150,8 +153,9 @@ private void processClassFileSafely(String packageName, File classFile, Predicat } } - private Optional> loadClassFromFile(String packageName, File classFile) { - String className = classFile.getName().substring(0, classFile.getName().length() - CLASS_FILE_SUFFIX.length()); + private Optional> loadClassFromFile(String packageName, Path classFile) { + String fileName = classFile.getFileName().toString(); + String className = fileName.substring(0, fileName.length() - CLASS_FILE_SUFFIX.length()); // Handle default package appropriately. String fqcn = StringUtils.isBlank(packageName) ? className : packageName + "." + className; @@ -159,7 +163,7 @@ private Optional> loadClassFromFile(String packageName, File classFile) return this.loadClass.apply(fqcn, getClassLoader()); } - private void handleInternalError(File classFile, Optional> clazz, InternalError ex) { + private void handleInternalError(Path classFile, Optional> clazz, InternalError ex) { if (MALFORMED_CLASS_NAME_ERROR_MESSAGE.equals(ex.getMessage())) { logMalformedClassName(classFile, clazz, ex); } @@ -168,24 +172,24 @@ private void handleInternalError(File classFile, Optional> clazz, Inter } } - private void handleThrowable(File classFile, Throwable throwable) { + private void handleThrowable(Path classFile, Throwable throwable) { rethrowIfBlacklisted(throwable); logGenericFileProcessingException(classFile, throwable); } - private void logMalformedClassName(File classFile, Optional> clazz, InternalError ex) { + private void logMalformedClassName(Path classFile, Optional> clazz, InternalError ex) { try { 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())); + () -> format("The java.lang.Class loaded from path [%s] has a malformed class name [%s].", + classFile.toAbsolutePath(), clazz.get().getName())); } else { - logWarning(ex, () -> format("The java.lang.Class loaded from file [%s] has a malformed class name.", - classFile.getAbsolutePath())); + logWarning(ex, () -> format("The java.lang.Class loaded from path [%s] has a malformed class name.", + classFile.toAbsolutePath())); } } catch (Throwable t) { @@ -194,9 +198,9 @@ private void logMalformedClassName(File classFile, Optional> clazz, Int } } - 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 void logGenericFileProcessingException(Path classFile, Throwable throwable) { + logWarning(throwable, () -> format("Failed to load java.lang.Class for path [%s] during classpath scanning.", + classFile.toAbsolutePath())); } private String appendSubpackageName(String packageName, String subpackageName) { @@ -213,29 +217,35 @@ private static void assertPackageNameIsPlausible(String packageName) { "package name must not contain only whitespace"); } - private static boolean isNotPackageInfo(File file) { - return !"package-info.class".equals(file.getName()); + private static boolean isNotPackageInfo(Path path) { + return !path.endsWith("package-info.class"); } - private static boolean isClassFile(File file) { - return file.isFile() && file.getName().endsWith(CLASS_FILE_SUFFIX); + private static boolean isClassFile(Path path) { + return Files.isRegularFile(path) && path.getFileName().toString().endsWith(CLASS_FILE_SUFFIX); } private static String packagePath(String packageName) { return packageName.replace('.', '/'); } - private List getSourceDirsForPackage(String packageName) { + private List getSourceDirsForPackage(String packageName) { try { Enumeration resources = getClassLoader().getResources(packagePath(packageName)); - List dirs = new ArrayList<>(); + List dirs = new ArrayList<>(); while (resources.hasMoreElements()) { 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; } catch (Exception ex) { + logWarning(ex, () -> "Error reading directories from class loader for package " + packageName); return Collections.emptyList(); } } diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java index 8bc55a1a57f..2c336cb21e5 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java @@ -23,6 +23,7 @@ import java.lang.reflect.Member; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -448,7 +449,7 @@ public static Set getAllClasspathRootDirectories() { // @formatter:on } - public static List> findAllClassesInClasspathRoot(File root, Predicate> classTester) { + public static List> findAllClassesInClasspathRoot(Path root, Predicate> classTester) { return classpathScanner.scanForClassesInClasspathRoot(root, classTester); } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathRootSelector.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathRootSelector.java index c16afe1bd22..68b21160d97 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathRootSelector.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathRootSelector.java @@ -12,7 +12,7 @@ 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.util.ToStringBuilder; @@ -30,16 +30,16 @@ @API(Experimental) public class ClasspathRootSelector implements DiscoverySelector { - private final File classpathRoot; + private final URI classpathRoot; - ClasspathRootSelector(File classpathRoot) { + ClasspathRootSelector(URI 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; } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/DiscoverySelectors.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/DiscoverySelectors.java index e09c5e8257a..dc6756b3ea4 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/DiscoverySelectors.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/DiscoverySelectors.java @@ -188,6 +188,7 @@ public static List selectClasspathRoots(Set directo // @formatter:off return directories.stream() .filter(File::isDirectory) + .map(File::toURI) .map(ClasspathRootSelector::new) .collect(toList()); // @formatter:on diff --git a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClasspathRootSelectorResolver.java b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClasspathRootSelectorResolver.java index afea0ba93d4..35aab79538a 100644 --- a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClasspathRootSelectorResolver.java +++ b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClasspathRootSelectorResolver.java @@ -12,6 +12,8 @@ import static org.junit.platform.commons.util.ReflectionUtils.findAllClassesInClasspathRoot; +import java.nio.file.Paths; + import org.junit.platform.engine.discovery.ClasspathRootSelector; /** @@ -25,7 +27,8 @@ class ClasspathRootSelectorResolver extends DiscoverySelectorResolver> thisClassOnly = clazz -> clazz == ClasspathScannerTests.class; - File root = getTestClasspathRoot(); + Path root = getTestClasspathRoot(); List> classes = classpathScanner.scanForClassesInClasspathRoot(root, thisClassOnly); assertSame(ClasspathScannerTests.class, classes.get(0)); } @@ -205,7 +206,7 @@ private boolean inDefaultPackage(Class clazz) { @Test void findAllClassesInClasspathRootWithFilter() throws Exception { - File root = getTestClasspathRoot(); + Path root = getTestClasspathRoot(); List> classes = classpathScanner.scanForClassesInClasspathRoot(root, clazz -> true); assertThat(classes.size()).isGreaterThanOrEqualTo(20); @@ -221,7 +222,7 @@ void findAllClassesInClasspathRootForNullRoot() throws Exception { @Test void findAllClassesInClasspathRootForNonExistingRoot() throws Exception { assertThrows(PreconditionViolationException.class, - () -> classpathScanner.scanForClassesInClasspathRoot(new File("does_not_exist"), clazz -> true)); + () -> classpathScanner.scanForClassesInClasspathRoot(Paths.get("does_not_exist"), clazz -> true)); } @Test @@ -230,9 +231,9 @@ void findAllClassesInClasspathRootForNullClassFilter() throws Exception { () -> classpathScanner.scanForClassesInClasspathRoot(getTestClasspathRoot(), null)); } - private File getTestClasspathRoot() throws Exception { + private Path getTestClasspathRoot() throws Exception { URL location = getClass().getProtectionDomain().getCodeSource().getLocation(); - return new File(location.toURI()); + return Paths.get(location.toURI()); } class MemberClassToBeFound { diff --git a/platform-tests/src/test/java/org/junit/platform/console/tasks/DiscoveryRequestCreatorTests.java b/platform-tests/src/test/java/org/junit/platform/console/tasks/DiscoveryRequestCreatorTests.java index e9b7d9f30b5..3de71a0ff22 100644 --- a/platform-tests/src/test/java/org/junit/platform/console/tasks/DiscoveryRequestCreatorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/console/tasks/DiscoveryRequestCreatorTests.java @@ -18,6 +18,7 @@ import java.io.File; import java.lang.reflect.Method; +import java.net.URI; import java.util.List; import org.junit.jupiter.api.Test; @@ -84,7 +85,7 @@ public void convertsAllOptionWithoutExplicitRootDirectories() { List classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class); // @formatter:off assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot) - .hasAtLeastOneElementOfType(File.class) + .hasAtLeastOneElementOfType(URI.class) .doesNotContainNull(); // @formatter:on } @@ -99,7 +100,7 @@ public void convertsAllOptionWithExplicitRootDirectories() { List classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class); // @formatter:off assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot) - .containsExactly(new File("."), new File("..")); + .containsExactly(new File(".").toURI(), new File("..").toURI()); // @formatter:on } @@ -113,7 +114,7 @@ public void convertsAllOptionWithAdditionalClasspathEntries() { List classpathRootSelectors = request.getSelectorsByType(ClasspathRootSelector.class); // @formatter:off assertThat(classpathRootSelectors).extracting(ClasspathRootSelector::getClasspathRoot) - .contains(new File("."), new File("..")); + .contains(new File(".").toURI(), new File("..").toURI()); // @formatter:on } diff --git a/platform-tests/src/test/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilderTests.java b/platform-tests/src/test/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilderTests.java index c6c41de4321..2f90379c031 100644 --- a/platform-tests/src/test/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilderTests.java +++ b/platform-tests/src/test/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilderTests.java @@ -166,7 +166,8 @@ public void availableFoldersAreStoredInDiscoveryRequest() throws Exception { // @formatter:on List 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()); }