Skip to content

Commit 1b99fdc

Browse files
holly-cumminsgsmet
authored andcommitted
Don't sock away a deployment classloader until we know we need it, and don't keep it if we know it's bad
(cherry picked from commit 756071f)
1 parent c198007 commit 1b99fdc

File tree

6 files changed

+36
-6
lines changed

6 files changed

+36
-6
lines changed

core/deployment/src/main/java/io/quarkus/deployment/dev/testing/JunitTestRunner.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,7 @@ public Runnable prepare() {
153153
ClassLoader old = Thread.currentThread().getContextClassLoader();
154154
QuarkusClassLoader tcl = testApplication.createDeploymentClassLoader();
155155
deploymentClassLoader = tcl;
156-
if (firstDeploymentClassLoader == null) {
157-
firstDeploymentClassLoader = deploymentClassLoader;
158-
}
156+
159157
LogCapturingOutputFilter logHandler = new LogCapturingOutputFilter(testApplication, true, true,
160158
TestSupport.instance()
161159
.get()::isDisplayTestOutput);
@@ -724,9 +722,13 @@ private DiscoveryResult discoverTestClasses() {
724722
ClassLoader classLoaderForLoadingTests;
725723
Closeable classLoaderToClose = null;
726724
ClassLoader orig = Thread.currentThread().getContextClassLoader();
725+
// JUnitTestRunner is loaded with an augmentation classloader which does not have visibility of FacadeClassLoader, but the deployment classloader can see it
726+
// We need a consistent classloader or we leak curated applications, so use a static classloader we stashed away
727+
// In a multi-module project we will have several deployment classloaders, but it's ok
728+
if (firstDeploymentClassLoader == null) {
729+
firstDeploymentClassLoader = deploymentClassLoader;
730+
}
727731
try {
728-
// JUnitTestRunner is loaded with an augmentation classloader which does not have visibility of FacadeClassLoader, but the deployment classloader can see it
729-
// We need a consistent classloader or we leak curated applications, so use a static classloader we stashed away
730732
Class fclClazz = firstDeploymentClassLoader.loadClass(FACADE_CLASS_LOADER_NAME);
731733
Constructor constructor = fclClazz.getConstructor(ClassLoader.class, boolean.class, CuratedApplication.class,
732734
Map.class,
@@ -746,6 +748,8 @@ private DiscoveryResult discoverTestClasses() {
746748
.setContextClassLoader(classLoaderForLoadingTests);
747749
} catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException | InstantiationException
748750
| InvocationTargetException e) {
751+
// If the first deployment classloader cannot load a facade classloader, don't keep using it, let the next module provide one
752+
firstDeploymentClassLoader = null;
749753
// This is fine, and usually just means that test-framework/junit5 isn't one of the project dependencies
750754
// In that case, fallback to loading classes as we normally would, using a TCCL
751755
log.debug(
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project>
3+
<modelVersion>4.0.0</modelVersion>
4+
5+
<parent>
6+
<groupId>org.acme</groupId>
7+
<artifactId>quarkus-quickstart-multimodule-parent</artifactId>
8+
<version>1.0-SNAPSHOT</version>
9+
</parent>
10+
<groupId>org.acme</groupId>
11+
<artifactId>quarkus-quickstart-multimodule-dependency</artifactId>
12+
<version>1.0-SNAPSHOT</version>
13+
14+
</project>

integration-tests/maven/src/test/resources-filtered/projects/multimodule/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
2121
</properties>
2222
<modules>
23+
<module>dependency</module>
2324
<module>rest</module>
2425
<module>html</module>
2526
<module>runner</module>
@@ -63,6 +64,11 @@
6364
<artifactId>quarkus-quickstart-multimodule-rest</artifactId>
6465
<version>1.0-SNAPSHOT</version>
6566
</dependency>
67+
<dependency>
68+
<groupId>org.acme</groupId>
69+
<artifactId>quarkus-quickstart-multimodule-dependency</artifactId>
70+
<version>1.0-SNAPSHOT</version>
71+
</dependency>
6672
</dependencies>
6773
</dependencyManagement>
6874

integration-tests/maven/src/test/resources-filtered/projects/multimodule/rest/src/test/java/org/acme/test/SimpleTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import org.junit.jupiter.api.Test;
55

66
public class SimpleTest {
7-
7+
88
@Test
99
public void testNothing() {
1010
Assertions.assertTrue(true);

integration-tests/maven/src/test/resources-filtered/projects/multimodule/runner/pom.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424
<groupId>org.acme</groupId>
2525
<artifactId>quarkus-quickstart-multimodule-rest</artifactId>
2626
</dependency>
27+
<dependency>
28+
<groupId>org.acme</groupId>
29+
<artifactId>quarkus-quickstart-multimodule-dependency</artifactId>
30+
</dependency>
2731
<dependency>
2832
<groupId>io.quarkus</groupId>
2933
<artifactId>quarkus-junit5</artifactId>

test-framework/junit5/src/main/java/io/quarkus/test/junit/classloading/FacadeClassLoader.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ public FacadeClassLoader(ClassLoader parent) {
113113
this(parent, false, null, null, null, System.getProperty("java.class.path"));
114114
}
115115

116+
// Also called reflectively by JUnitTestRunner
116117
public FacadeClassLoader(ClassLoader parent, boolean isAuxiliaryApplication, CuratedApplication curatedApplication,
117118
final Map<String, String> profileNames,
118119
final Set<String> quarkusTestClasses, final String classesPath) {
@@ -563,6 +564,7 @@ private QuarkusClassLoader getOrCreateRuntimeClassLoader(String key, Class<?> re
563564
// If the try block fails, this would be null, but there's no catch, so we'd never get to this code
564565
QuarkusClassLoader loader = startupAction.getClassLoader();
565566

567+
// Make sure that our new classloader has config on it; this is a bit of a scattergun approach to setting config, but it helps cover most paths
566568
Class<?> configProviderResolverClass = loader.loadClass(ConfigProviderResolver.class.getName());
567569

568570
Class<?> testConfigProviderResolverClass = loader.loadClass(QuarkusTestConfigProviderResolver.class.getName());

0 commit comments

Comments
 (0)