Skip to content

Commit

Permalink
Correctly set the resource key in cases where the first class we look…
Browse files Browse the repository at this point in the history 
… at has a resource on it
  • Loading branch information
@holly-cummins
holly-cummins committed Jan 21, 2025
1 parent 632914f commit 55cd78c
Showing 1 changed file with 47 additions and 43 deletions.
90 changes: 47 additions & 43 deletions core/deployment/src/main/java/io/quarkus/test/junit/classloading/FacadeClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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
/*
Expand Down

0 comments on commit 55cd78c

Please sign in to comment.