From 55cd78c25b03cb4db59d0c980022247cdecf6100 Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Tue, 21 Jan 2025 20:27:34 +0000 Subject: [PATCH] Correctly set the resource key in cases where the first class we look at has a resource on it --- .../junit/classloading/FacadeClassLoader.java | 90 ++++++++++--------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/test/junit/classloading/FacadeClassLoader.java b/core/deployment/src/main/java/io/quarkus/test/junit/classloading/FacadeClassLoader.java index 74077a1599a849..0ac7ecd85035bd 100644 --- a/core/deployment/src/main/java/io/quarkus/test/junit/classloading/FacadeClassLoader.java +++ b/core/deployment/src/main/java/io/quarkus/test/junit/classloading/FacadeClassLoader.java @@ -223,9 +223,6 @@ public Class loadClass(String name) throws ClassNotFoundException { // TODO should we use JUnit's AnnotationSupport? It searches class hierarchies. Unless we have a good reason not to use it, perhaps we should? // See, for example, https://github.com/marcphilipp/gradle-sandbox/blob/baaa1972e939f5817f54a3d287611cef0601a58d/classloader-per-test-class/src/test/java/org/example/ClassLoaderReplacingLauncherSessionListener.java#L23-L44 - Arrays.stream(fromCanary.getAnnotations()) - .map(Annotation::annotationType) - .forEach(o -> System.out.println("annotation tyoe " + o)); String profileName = "no-profile"; Class profile = null; @@ -244,10 +241,6 @@ public Class loadClass(String name) throws ClassNotFoundException { } else { // TODO JUnitRunner already worked all this out for the dev mode case, could we share some logic? - System.out.println( - "HOLLY annotations is " + Arrays.toString(Arrays.stream(fromCanary.getAnnotations()) - .toArray())); - // TODO make this test cleaner + more rigorous // A Quarkus Test could be annotated with @QuarkusTest or with @ExtendWith[... QuarkusTestExtension.class ] or @RegisterExtension // An @interface isn't a quarkus test, and doesn't want its own application; to detect it, just check if it has a superclass - except that fails for things whose superclass isn't on the classpath, like javax.tools subclasses @@ -390,53 +383,41 @@ private boolean registersQuarkusTestExtension(Class fromCanary) { private QuarkusClassLoader getQuarkusClassLoader(String profileKey, Class requiredTestClass, Class profile) { try { - String resourceKey; + AppMakerHelper.DumbHolder holder; + String key; // We cannot directly access TestResourceUtil as long as we're in the core module, but the app classloaders can. // But, chicken-and-egg, we may not have an app classloader yet. However, if we don't, we won't need to worry about restarts, but this instance clearly cannot need a restart - // TODO make sure this magic string is the same as what test resource manager uses, even though the classes can't see each other if (keyMakerClassLoader == null) { - resourceKey = ""; - } else { - Method method = Class - .forName("io.quarkus.test.junit.TestResourceUtil", true, keyMakerClassLoader) // TODO use class, not string, but that would need us to be in a different module - .getMethod("getReloadGroupIdentifier", Class.class, Class.class); - - // TODO this is kind of annoying, can we find a nicer way? - // The resource checks assume that there's a useful TCCL and load the class with that TCCL to do reference equality checks and casting against resource classes - // That does mean we potentially load the test class three times, if there's resources - ClassLoader original = Thread.currentThread().getContextClassLoader(); - try { - Thread.currentThread() - .setContextClassLoader(keyMakerClassLoader); - - // we reload the test resources (and thus the application) if we changed test class and the new test class is not a nested class, and if we had or will have per-test test resources - resourceKey = (String) method.invoke(null, requiredTestClass, profile); // TestResourceUtil.getResourcesKey(requiredTestClass); - } finally { - Thread.currentThread().setContextClassLoader(original); - } - } - - final String key = profileKey + resourceKey; - System.out.println("HOLLY With resources, key is " + key); + // Making a classloader uses the profile key to look up a curated application + holder = makeClassLoader(profileKey, requiredTestClass, profile); + keyMakerClassLoader = holder.startupAction() + .getClassLoader(); - AppMakerHelper.DumbHolder holder = runtimeClassLoaders.get(key); - System.out.println("HOLLY seen this key before " + holder); + // Now make sure to get the right key, so that the next test along can compare to see if it needs a restart + final String resourceKey = getResourceKey(requiredTestClass, profile); + key = profileKey + resourceKey; + } else { + final String resourceKey = getResourceKey(requiredTestClass, profile); - if (holder == null) { - // TODO can we make this less confusing? + // The resource key might be null, and that's ok + key = profileKey + resourceKey; + System.out.println("HOLLY With resources, key is " + key); - // Making a classloader uses the profile key to look up a curated application - holder = makeClassLoader(profileKey, requiredTestClass, profile); + holder = runtimeClassLoaders.get(key); + System.out.println("HOLLY seen this key before " + holder); - runtimeClassLoaders.put(key, holder); + if (holder == null) { + // TODO can we make this less confusing? + // Making a classloader uses the profile key to look up a curated application + holder = makeClassLoader(profileKey, requiredTestClass, profile); + } } - if (keyMakerClassLoader == null) { - keyMakerClassLoader = holder.startupAction() - .getClassLoader(); - } + // If we didn't have a classloader and didn't get a resource key + + runtimeClassLoaders.put(key, holder); return holder.startupAction() .getClassLoader(); @@ -448,6 +429,29 @@ private QuarkusClassLoader getQuarkusClassLoader(String profileKey, Class requir } } + private String getResourceKey(Class requiredTestClass, Class profile) + throws NoSuchMethodException, ClassNotFoundException, IllegalAccessException, InvocationTargetException { + String resourceKey; + Method method = Class + .forName("io.quarkus.test.junit.TestResourceUtil", true, keyMakerClassLoader) // TODO use class, not string, but that would need us to be in a different module + .getMethod("getReloadGroupIdentifier", Class.class, Class.class); + + // TODO this is kind of annoying, can we find a nicer way? + // The resource checks assume that there's a useful TCCL and load the class with that TCCL to do reference equality checks and casting against resource classes + // That does mean we potentially load the test class three times, if there's resources + ClassLoader original = Thread.currentThread().getContextClassLoader(); + try { + Thread.currentThread() + .setContextClassLoader(keyMakerClassLoader); + + // we reload the test resources (and thus the application) if we changed test class and the new test class is not a nested class, and if we had or will have per-test test resources + resourceKey = (String) method.invoke(null, requiredTestClass, profile); // TestResourceUtil.getResourcesKey(requiredTestClass); + } finally { + Thread.currentThread().setContextClassLoader(original); + } + return resourceKey; + } + // TODO copied from IntegrationTestUtil - if this was in that module, could just use directly // TODO delete this, as it seems unused /*