Ensure FacadeClassLoader is not set as the TCCL during continuous test execution, initialise config on the CL created by JUnitTestRunner

Author: holly-cummins

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

@@ -98,7 +98,7 @@ public class JunitTestRunner {
     public static final DotName TESTABLE = DotName.createSimple(Testable.class.getName());
     public static final DotName NESTED = DotName.createSimple(Nested.class.getName());
     private static final String ARCHUNIT_FIELDSOURCE_FQCN = "com.tngtech.archunit.junit.FieldSource";
-    public static final String FACADE_CLASS_LOADER_NAME = "io.quarkus.test.junit.classloading.FacadeClassLoader";
+    private static final String FACADE_CLASS_LOADER_NAME = "io.quarkus.test.junit.classloading.FacadeClassLoader";
 
     private final long runId;
     private final DevModeContext.ModuleInfo moduleInfo;

integration-tests/command-mode/src/test/java/org/acme/MainContinuousTestingTest.java

@@ -4,20 +4,18 @@
 import org.jboss.shrinkwrap.api.asset.StringAsset;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import io.quarkus.test.ContinuousTestingTestUtils;
 import io.quarkus.test.ContinuousTestingTestUtils.TestStatus;
 import io.quarkus.test.QuarkusDevModeTest;
 
-@Disabled("See https://github.com/quarkusio/quarkus/issues/49780#issuecomment-3265067545")
 public class MainContinuousTestingTest {
     @RegisterExtension
     final static QuarkusDevModeTest TEST = new QuarkusDevModeTest()
             .withApplicationRoot(jar -> jar
-                    .addClass(Main.class)
+                    // Files from this module are added automatically, so no need to add Main.class
                     .add(new StringAsset(ContinuousTestingTestUtils.appProperties()), "application.properties"))
             .setTestArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
                     .addClass(MainTest.class));

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

@@ -139,8 +139,8 @@ public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext con
             return ConditionEvaluationResult.enabled("Quarkus Test Profile tags only affect classes");
         }
 
-        // At this point, the TCCL is usually the FacadeClassLoader, but sometimes it's a deployment classloader (for multimodule tests), or the runtime classloader (for nested tests)
-        // Getting back to the FacadeClassLoader is non-trivial. We can't use the singleton on the class, because we will be accessing it from different classloaders.
+        // At this point, the TCCL is sometimes a deployment classloader (for multimodule tests), or the runtime classloader (for nested tests), and sometimes a FacadeClassLoader in continuous cases
+        // Getting back to a FacadeClassLoader is non-trivial. We can't use the singleton on the class, because we will be accessing it from different classloaders.
         // We can't have a hook back from the runtime classloader to the facade classloader, because
         // when evaluating execution conditions for native tests, the test will have been loaded with the system classloader, not the runtime classloader.
         // The one classloader we can reliably get to when evaluating test execution is the system classloader, so hook our config on that.

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

@@ -113,6 +113,8 @@ public final class FacadeClassLoader extends ClassLoader implements Closeable {
     private final boolean isAuxiliaryApplication;
     private QuarkusClassLoader keyMakerClassLoader;
 
+    private final boolean isContinuousTesting;
+
     public FacadeClassLoader(ClassLoader parent) {
         this(parent, false, null, null, null, System.getProperty("java.class.path"));
     }
@@ -122,9 +124,19 @@ public FacadeClassLoader(ClassLoader parent, boolean isAuxiliaryApplication, Cur
             final Map<String, String> profileNames,
             final Set<String> quarkusTestClasses, final String classesPath) {
         super(parent);
+        // Note that in normal testing, the parent is the system classloader, and in continuous testing, the parent is a quarkus classloader
+        // It would be nice to resolve that inconsistency, but I'm not sure it's very possible
 
-        // Don't make a no-profile curated application, since our caller had one already
-        curatedApplications.put(getProfileKey(null), curatedApplication);
+        if (quarkusTestClasses != null) {
+            isContinuousTesting = true;
+        } else {
+            isContinuousTesting = false;
+        }
+
+        // Don't make a no-profile curated application if our caller had one already
+        if (curatedApplication != null) {
+            curatedApplications.put(getProfileKey(null), curatedApplication);
+        }
 
         this.quarkusTestClasses = quarkusTestClasses;
         this.isAuxiliaryApplication = isAuxiliaryApplication;
@@ -227,6 +239,20 @@ public FacadeClassLoader(ClassLoader parent, boolean isAuxiliaryApplication, Cur
             // We set it to null so we know not to look in it
             this.profiles = null;
         }
+
+        // In the case where a QuarkusMainTest is present, but not a QuarkusTest, tests will be loaded and run using
+        // the parent classloader. In continuous testing mode, that will be a Quarkus classloader and it will not have
+        // the test config on it, so get that classloader test-ready.
+
+        // It would be nice to do this without the guard, but doing this on the normal path causes us to write something we don't want
+        if (isContinuousTesting) {
+            try {
+                initialiseTestConfig(parent);
+            } catch (ClassNotFoundException | InvocationTargetException | NoSuchMethodException | InstantiationException
+                    | IllegalAccessException e) {
+                throw new RuntimeException(e);
+            }
+        }
     }
 
     @Override
@@ -254,7 +280,7 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
         try {
 
             Class<?> profile = null;
-            if (profiles != null && !isServiceLoaderMechanism) {
+            if (isContinuousTesting && !isServiceLoaderMechanism) {
                 isQuarkusTest = quarkusTestClasses.contains(name);
 
                 profile = profiles.get(name);
@@ -551,6 +577,14 @@ private QuarkusClassLoader getOrCreateRuntimeClassLoader(String key, Class<?> re
         // If the try block fails, this would be null, but there's no catch, so we'd never get to this code
         QuarkusClassLoader loader = startupAction.getClassLoader();
 
+        initialiseTestConfig(loader);
+
+        return loader;
+
+    }
+
+    private static void initialiseTestConfig(ClassLoader loader) throws ClassNotFoundException, InstantiationException,
+            IllegalAccessException, InvocationTargetException, NoSuchMethodException {
         // 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
         Class<?> configProviderResolverClass = loader.loadClass(ConfigProviderResolver.class.getName());
 
@@ -560,9 +594,6 @@ private QuarkusClassLoader getOrCreateRuntimeClassLoader(String key, Class<?> re
 
         configProviderResolverClass.getDeclaredMethod("setInstance", configProviderResolverClass)
                 .invoke(null, testConfigProviderResolver);
-
-        return loader;
-
     }
 
     public boolean isServiceLoaderMechanism() {

test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/CustomLauncherInterceptor.java

@@ -31,13 +31,15 @@ public void launcherSessionOpened(LauncherSession session) {
          * However, the Eclipse runner calls this twice, and the second invocation happens after discovery,
          * which means there is no one to unset the TCCL. That breaks integration tests, so we
          * need to add an ugly guard to not adjust the TCCL the second time round in that scenario.
+         *
+         * There is a similar issue with continuous testing, causing us to go into tests with the FacadeClassLoader set.
+         * That's not the right CL, so in those cases skip setting it.
+         *
          * We do not do any classloading dance for prod mode tests.
          */
-        boolean isEclipse = System.getProperty("sun.java.command") != null
-                && System.getProperty("sun.java.command").contains("JUnit5TestLoader");
-        boolean shouldSkipSettingTCCL = isEclipse && discoveryStarted;
+        boolean shouldSetTCCL = !discoveryStarted;
 
-        if (!isProductionModeTests() && !shouldSkipSettingTCCL) {
+        if (!isProductionModeTests() && shouldSetTCCL) {
             actuallyIntercept();
         }