Skip to content

Commit

Permalink
Fix package path computation in ClasspathScanner
Browse files Browse the repository at this point in the history
Fix `getRootUrisForPackage()` in class `ClasspathScanner` by looking for two
"wordings" of a package name.

For example, package `foo.bar` is expanded to `foo/bar` and `foo/bar/`.
The latter allows find package `foo.bar` in a module called `foo.bar`,
and not `foo`.
This commit also ensures, that there's no regression (compared to #2531)
by launching test runs using the standalone artifact with `--select-package`
and `--scan-class-path`.

Fixes #2500
Fixes #2600
Fixes #2612
  • Loading branch information
sormuras authored and marcphilipp committed May 13, 2021
1 parent 4b52cc7 commit cf9c8fd
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Enumeration;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Supplier;
Expand All @@ -49,6 +51,8 @@ class ClasspathScanner {
private static final Logger logger = LoggerFactory.getLogger(ClasspathScanner.class);

private static final char CLASSPATH_RESOURCE_PATH_SEPARATOR = '/';
private static final String CLASSPATH_RESOURCE_PATH_SEPARATOR_STRING = String.valueOf(
CLASSPATH_RESOURCE_PATH_SEPARATOR);
private static final char PACKAGE_SEPARATOR_CHAR = '.';
private static final String PACKAGE_SEPARATOR_STRING = String.valueOf(PACKAGE_SEPARATOR_CHAR);

Expand All @@ -74,7 +78,8 @@ List<Class<?>> scanForClassesInPackage(String basePackageName, ClassFilter class
Preconditions.notNull(classFilter, "classFilter must not be null");
basePackageName = basePackageName.trim();

return findClassesForUris(getRootUrisForPackage(basePackageName), basePackageName, classFilter);
return findClassesForUris(getRootUrisForPackageNameOnClassPathAndModulePath(basePackageName), basePackageName,
classFilter);
}

List<Class<?>> scanForClassesInClasspathRoot(URI root, ClassFilter classFilter) {
Expand Down Expand Up @@ -210,12 +215,29 @@ private ClassLoader getClassLoader() {
return this.classLoaderSupplier.get();
}

private List<URI> getRootUrisForPackageNameOnClassPathAndModulePath(String basePackageName) {
Set<URI> uriSet = new LinkedHashSet<>(getRootUrisForPackage(basePackageName));
if (!basePackageName.isEmpty() && !basePackageName.endsWith(PACKAGE_SEPARATOR_STRING)) {
getRootUrisForPackage(basePackageName + PACKAGE_SEPARATOR_STRING).stream() //
.map(ClasspathScanner::removeTrailingClasspathResourcePathSeparator) //
.forEach(uriSet::add);
}
return new ArrayList<>(uriSet);
}

private static URI removeTrailingClasspathResourcePathSeparator(URI uri) {
String string = uri.toString();
if (string.endsWith(CLASSPATH_RESOURCE_PATH_SEPARATOR_STRING)) {
return URI.create(string.substring(0, string.length() - 1));
}
return uri;
}

private static String packagePath(String packageName) {
if (packageName.isEmpty()) {
return "";
}
String path = packageName.replace(PACKAGE_SEPARATOR_CHAR, CLASSPATH_RESOURCE_PATH_SEPARATOR);
return path + CLASSPATH_RESOURCE_PATH_SEPARATOR;
return packageName.replace(PACKAGE_SEPARATOR_CHAR, CLASSPATH_RESOURCE_PATH_SEPARATOR);
}

private List<URI> getRootUrisForPackage(String basePackageName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
import org.opentest4j.TestAbortedException;

import platform.tooling.support.Helper;
import platform.tooling.support.Request;
Expand Down Expand Up @@ -105,6 +106,73 @@ void test() throws IOException {

@Test
@Order(3)
void testOnJava8() throws IOException {
var result = Request.builder() //
.setTool(new Java()) //
.setJavaHome(Helper.getJavaHome("8").orElseThrow(TestAbortedException::new)) //
.setProject("standalone") //
.addArguments("--show-version") //
.addArguments("-enableassertions") //
.addArguments("-Djava.util.logging.config.file=logging.properties") //
.addArguments("-jar", MavenRepo.jar("junit-platform-console-standalone")) //
.addArguments("--scan-class-path") //
.addArguments("--disable-banner") //
.addArguments("--include-classname", "standalone.*") //
.addArguments("--classpath", "bin").build() //
.run(false);

assertEquals(1, result.getExitCode(), String.join("\n", result.getOutputLines("out")));

var workspace = Request.WORKSPACE.resolve("standalone");
var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt"));
var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err.txt"));
assertLinesMatch(expectedOutLines, result.getOutputLines("out"));
assertLinesMatch(expectedErrLines, result.getOutputLines("err"));

var jupiterVersion = Helper.version("junit-jupiter-engine");
var vintageVersion = Helper.version("junit-vintage-engine");
assertTrue(result.getOutput("err").contains("junit-jupiter"
+ " (group ID: org.junit.jupiter, artifact ID: junit-jupiter-engine, version: " + jupiterVersion));
assertTrue(result.getOutput("err").contains("junit-vintage"
+ " (group ID: org.junit.vintage, artifact ID: junit-vintage-engine, version: " + vintageVersion));
}

@Test
@Order(3)
// https://github.com/junit-team/junit5/issues/2600
void testOnJava8SelectPackage() throws IOException {
var result = Request.builder() //
.setTool(new Java()) //
.setJavaHome(Helper.getJavaHome("8").orElseThrow(TestAbortedException::new)) //
.setProject("standalone") //
.addArguments("--show-version") //
.addArguments("-enableassertions") //
.addArguments("-Djava.util.logging.config.file=logging.properties") //
.addArguments("-jar", MavenRepo.jar("junit-platform-console-standalone")) //
.addArguments("--select-package", "standalone") //
.addArguments("--disable-banner") //
.addArguments("--include-classname", "standalone.*") //
.addArguments("--classpath", "bin").build() //
.run(false);

assertEquals(1, result.getExitCode(), String.join("\n", result.getOutputLines("out")));

var workspace = Request.WORKSPACE.resolve("standalone");
var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt"));
var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err.txt"));
assertLinesMatch(expectedOutLines, result.getOutputLines("out"));
assertLinesMatch(expectedErrLines, result.getOutputLines("err"));

var jupiterVersion = Helper.version("junit-jupiter-engine");
var vintageVersion = Helper.version("junit-vintage-engine");
assertTrue(result.getOutput("err").contains("junit-jupiter"
+ " (group ID: org.junit.jupiter, artifact ID: junit-jupiter-engine, version: " + jupiterVersion));
assertTrue(result.getOutput("err").contains("junit-vintage"
+ " (group ID: org.junit.vintage, artifact ID: junit-vintage-engine, version: " + vintageVersion));
}

@Test
@Order(5)
@Disabled("https://github.com/junit-team/junit5/issues/1724")
void testWithJarredTestClasses() {
var root = Path.of("../../..");
Expand Down

0 comments on commit cf9c8fd

Please sign in to comment.