Skip to content

Commit

Permalink
Changes to make cloud functions and funqy cloud functions work, use p…
Browse files Browse the repository at this point in the history
…roper per-resource; also, proper implementation of test ordering (but tests for that disabled)
  • Loading branch information
holly-cummins committed Aug 15, 2024
1 parent a0a429e commit e05fa42
Show file tree
Hide file tree
Showing 10 changed files with 267 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ protected PrepareResult v2createAugmentor(CuratedApplication curatedApplication,
// Note that curated application cannot be re-used between restarts, so this application
// should have been freshly created
// TODO maybe don't even accept one?
public QuarkusClassLoader getStartupAction(Class testClass, CuratedApplication curatedApplication,
public DumbHolder getStartupAction(Class testClass, CuratedApplication curatedApplication,
boolean isContinuousTesting, Class ignoredProfile)
throws Exception {

Expand All @@ -557,14 +557,26 @@ public QuarkusClassLoader getStartupAction(Class testClass, CuratedApplication c

testHttpEndpointProviders = TestHttpEndpointProvider.load();

System.out.println("HOLLY about to make app for " + testClass);
StartupAction startupAction = augmentAction.createInitialRuntimeApplication();
try {
System.out.println("HOLLY about to make app for " + testClass);
StartupAction startupAction = augmentAction.createInitialRuntimeApplication();

// TODO this seems to be safe to do because the classloaders are the same
// TODO not doing it startupAction.store();
System.out.println("HOLLY did store " + startupAction);
return new DumbHolder(startupAction, result);
} catch (RuntimeException e) {
// Errors at this point just get reported as org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-jupiter' failed to discover tests
// Give a little help to debuggers
System.out.println("HOLLY IT ALL WENT WRONG + + e" + e);
e.printStackTrace();
throw e;

// TODO this seems to be safe to do because the classloaders are the same
// TODO not doing it startupAction.store();
System.out.println("HOLLY did store " + startupAction);
return (QuarkusClassLoader) startupAction.getClassLoader();
}

}

record DumbHolder(StartupAction startupAction, PrepareResult prepareResult) {
}

// public QuarkusClassLoader doJavaStart(PathList location, CuratedApplication curatedApplication, boolean isContinuousTesting)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@
import java.io.Closeable;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand All @@ -21,6 +26,7 @@
import io.quarkus.bootstrap.app.CuratedApplication;
import io.quarkus.bootstrap.classloading.QuarkusClassLoader;
import io.quarkus.runtime.LaunchMode;
import io.quarkus.test.junit.QuarkusTestProfile;

/**
* JUnit has many interceptors and listeners, but it does not allow us to intercept test discovery in a fine-grained way that
Expand Down Expand Up @@ -51,7 +57,7 @@ public class FacadeClassLoader extends ClassLoader implements Closeable {

// TODO does this need to be a thread safe maps?
private final Map<String, CuratedApplication> curatedApplications = new HashMap<>();
private final Map<String, QuarkusClassLoader> runtimeClassLoaders = new HashMap<>();
private final Map<String, AppMakerHelper.DumbHolder> runtimeClassLoaders = new HashMap<>();
private final ClassLoader parent;

/*
Expand Down Expand Up @@ -162,7 +168,7 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
}
}

System.out.println("HOLLY yay! did load " + name);
System.out.println("HOLLY canary did load " + name);
System.out.println("ANNOTATIONS " + Arrays.toString(fromCanary.getAnnotations()));
Arrays.stream(fromCanary.getAnnotations())
.map(Annotation::annotationType)
Expand All @@ -173,6 +179,7 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
if (profiles != null) {
// TODO the good is that we're re-using what JUnitRunner already worked out, the bad is that this is seriously clunky with multiple code paths, brittle information sharing ...
// TODO at the very least, should we have a test landscape holder class?
// TODO and what if JUnitRunner wasn't invoked, because this wasn't dev mode?!
isMainTest = quarkusMainTestClasses.contains(name);
// The JUnitRunner counts main tests as quarkus tests
isQuarkusTest = quarkusTestClasses.contains(name) && !isMainTest;
Expand Down Expand Up @@ -243,6 +250,13 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
.getParent());
Class thing = runtimeClassLoader.loadClass(name);
System.out.println("HOLLY did load " + thing);
System.out.println(
"HOLLY after cl TCCL is " + Thread.currentThread().getContextClassLoader() + " loaded " + name);
if (Thread.currentThread().getContextClassLoader() != this) {
// TODO this should not be needed, sort it out?
// TODO or is this actually a sensible tidy up at the end of creating the application? is leaving itself on the TCCL a sensible thing for create app to do?
Thread.currentThread().setContextClassLoader(this);
}
return thing;
} else {
System.out.println("HOLLY sending to " + super.getName());
Expand All @@ -268,23 +282,114 @@ private QuarkusClassLoader getQuarkusClassLoader(String key, Class requiredTestC
// boolean reloadTestResources = isNewTestClass && (hasPerTestResources || hasPerTestResources(extensionContext));
// if ((state == null && !failedBoot)) { // TODO never reload, as it will not work || wrongProfile || reloadTestResources) {

QuarkusClassLoader runtimeClassLoader = runtimeClassLoaders.get(key);
if (runtimeClassLoader == null) {
try {

AppMakerHelper.DumbHolder holder = runtimeClassLoaders.get(key);
if (holder == null) {
holder = makeClassLoader(key, requiredTestClass, profile);

runtimeClassLoaders.put(key, holder);

}

// TODO hack
// Once we've made the loader, we know that we have a startup action

// should be
// getAdditionalTestResources(profileInstance, startupAction.getClassLoader()),
// profileInstance != null && profileInstance.disableGlobalTestResources(),
// startupAction.getDevServicesProperties(), Optional.empty(), result.testClassLocation);

AppMakerHelper.PrepareResult result = holder.prepareResult();
QuarkusTestProfile profileInstance = holder.prepareResult().profileInstance;

System.out.println("HOLLY TCCL is " + Thread.currentThread().getContextClassLoader());

ClassLoader old = Thread.currentThread().getContextClassLoader();
// TODO do we need to set the TCCL, or just make at io.quarkus.test.common.TestResourceManager.getUniqueTestResourceClassEntries(TestResourceManager.java:281) not use the TCCL?

// TODO as a general safety thing for java.lang.ClassCircularityError should we take ourselves off the TCCL while creating the runtime?

Thread.currentThread().setContextClassLoader(holder.startupAction().getClassLoader());
boolean hasPerTestResources;
try {
runtimeClassLoader = makeClassLoader(key, requiredTestClass, profile);
// TODO diagnostic, assume everything is a per-test resource
boolean hasPerTestResources = profile != null;
if (!hasPerTestResources) {
runtimeClassLoaders.put(key, runtimeClassLoader);
}
} catch (Exception e) {
throw new RuntimeException(e);
Closeable testResourceManager = (Closeable) holder.startupAction()
.getClassLoader()
.loadClass("io.quarkus.test.common.TestResourceManager")// TODO use class, not string
.getConstructor(Class.class, Class.class, List.class, boolean.class, Map.class, Optional.class,
Path.class)
.newInstance(requiredTestClass,
profile != null ? profile : null,
getAdditionalTestResources(profileInstance, holder.startupAction()
.getClassLoader()),
profileInstance != null && profileInstance.disableGlobalTestResources(),
holder.startupAction()
.getDevServicesProperties(),
Optional.empty(), result.testClassLocation);
testResourceManager.getClass()
.getMethod("init", String.class)
.invoke(testResourceManager,
profile != null ? profile.getName() : null);

hasPerTestResources = (boolean) testResourceManager.getClass()
.getMethod("hasPerTestResources")
.invoke(testResourceManager);

System.out.println(
"HOLLY has per test resources " + requiredTestClass.getName() + ": " + hasPerTestResources);
} finally {
Thread.currentThread().setContextClassLoader(old); // Which is most likely 'this'
}

if (hasPerTestResources) {
return (QuarkusClassLoader) makeClassLoader(key, requiredTestClass, profile).startupAction().getClassLoader();
} else {
return (QuarkusClassLoader) holder.startupAction().getClassLoader();
}

} catch (Exception e) {
// Exceptions here get swallowed by the JUnit framework and we don't get any debug information unless we print it ourself
// TODO what's the best way to do this?
e.printStackTrace();
throw new RuntimeException(e);
}
return runtimeClassLoader;
}

private QuarkusClassLoader makeClassLoader(String key, Class requiredTestClass, Class profile) throws Exception {
// TODO copied from IntegrationTestUtil - if this was in that module, could just use directly
// TODO javadoc does not compile
/*
* Since {@link TestResourceManager} is loaded from the ClassLoader passed in as an argument,
* we need to convert the user input {@link QuarkusTestProfile.TestResourceEntry} into instances of
* {@link TestResourceManager.TestResourceClassEntry}
* that are loaded from that ClassLoader
*/
static <T> List<T> getAdditionalTestResources(
QuarkusTestProfile profileInstance, ClassLoader classLoader) {
if ((profileInstance == null) || profileInstance.testResources().isEmpty()) {
return Collections.emptyList();
}

try {
Constructor<?> testResourceClassEntryConstructor = Class
.forName("io.quarkus.test.common.TestResourceManager$TestResourceClassEntry", true, classLoader) // TODO use class, not string
.getConstructor(Class.class, Map.class, Annotation.class, boolean.class);

List<QuarkusTestProfile.TestResourceEntry> testResources = profileInstance.testResources();
List<T> result = new ArrayList<>(testResources.size());
for (QuarkusTestProfile.TestResourceEntry testResource : testResources) {
T instance = (T) testResourceClassEntryConstructor.newInstance(
Class.forName(testResource.getClazz().getName(), true, classLoader), testResource.getArgs(),
null, testResource.isParallel());
result.add(instance);
}

return result;
} catch (Exception e) {
throw new IllegalStateException("Unable to handle profile " + profileInstance.getClass(), e);
}
}

private AppMakerHelper.DumbHolder makeClassLoader(String key, Class requiredTestClass, Class profile) throws Exception {

// This interception is only actually needed in limited circumstances; when
// - running in normal mode
Expand Down Expand Up @@ -442,15 +547,17 @@ private QuarkusClassLoader makeClassLoader(String key, Class requiredTestClass,
// .getMode();
// TODO are all these args used?
// TODO we are hardcoding is continuous testing to the wrong value!
QuarkusClassLoader loader = appMakerHelper.getStartupAction(requiredTestClass,
AppMakerHelper.DumbHolder holder = appMakerHelper.getStartupAction(requiredTestClass,
curatedApplication, false, profile);

QuarkusClassLoader loader = (QuarkusClassLoader) holder.startupAction().getClassLoader();

// TODO is this a good idea?
// TODO without this, the parameter dev mode tests regress, but it feels kind of wrong - is there some use of TCCL in JUnitRunner we need to find
currentThread.setContextClassLoader(loader);

System.out.println("HOLLY did make a " + currentThread.getContextClassLoader());
return loader;
return holder;

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,40 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
if (name.startsWith(JAVA)) {
return parent.loadClass(name);
}

//
// || name.contains("io.quarkus.test.junit.QuarkusTestExtension")
// TODO this is a big hack, can we find another way?
// Test support classes will be loaded by the runtime, we want them to be loaded by the base runtime
// Otherwise they can't share state classes
// TODO would it work for the jar to configure them as parent first?
// TODO not sure the specific class extension helped, undo it - the extension the test sees gets loaded with the app classloader

// TODO needed to work around static variables in AbstractJvmQuarkusTestExtension

// if (name.equals("io.quarkus.test.junit.AbstractJvmQuarkusTestExtension")) {
// System.out.println("BOOM dumping " + name + " to system");
// return getSystemClassLoader().loadClass(name);
// }
//
// if (name.equals("io.quarkus.test.junit.QuarkusTestExtension")) {
// System.out.println("SPLAT dumping " + name + " to system");
// return getSystemClassLoader().loadClass(name);
// }

//
// && !name.equals("io.quarkus.test.junit.QuarkusTestExtension")
// && !name.equals("io.quarkus.test.junit.AbstractJvmQuarkusTestExtension")

// The Quarkus test extensions put classes into the JUnit store to communicate between instances
// If the test classes are loaded with the runtime classloader, some of these state classes will be as well,
// and so they cannot be shared. Hackily work around the problem by hardcoding an exception to child-first
if ((name.contains("io.quarkus.test.junit"))
&& this.getParent().getName().contains("Base Runtime")) {
System.out.println("Wheeeeee! dumping " + name + " to parent " + getParent());
return getParent().loadClass(name);
}

//even if the thread is interrupted we still want to be able to load classes
//if the interrupt bit is set then we clear it and restore it at the end
boolean interrupted = Thread.interrupted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@

import static java.util.Arrays.asList;

import java.lang.annotation.Annotation;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Stream;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.extension.Extension;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ParameterContext;
Expand All @@ -21,10 +18,16 @@ public class MyContextProvider implements TestTemplateInvocationContextProvider
@Override
public boolean supportsTestTemplate(ExtensionContext extensionContext) {
System.out.println("HOLLY checking test template");
Annotation[] myAnnotations = extensionContext.getClass().getAnnotations();
Assertions.assertTrue(Arrays.toString(myAnnotations).contains("AnnotationAddedByExtensionHAHAHANO"),
"The context provider does not see the annotation, only sees " + Arrays.toString(myAnnotations)
+ ". The classloader is " + this.getClass().getClassLoader());
// TODO In an ideal world, this template context would also see the updated class. At the moment it doesn't,
// which can be confirmed by uncommenting the following assertion. This class is loaded with an augmentation
// classloader, and the test class gets a deployment classloader

// Annotation[] myAnnotations = extensionContext.getRequiredTestClass().getAnnotations();
// Assertions.assertTrue(Arrays.toString(myAnnotations).contains("AnnotationAddedByExtension"),
// "The templating context provider does not see the annotation, only sees " + Arrays.toString(myAnnotations)
// + ". The classloader of the checking class is " + this.getClass().getClassLoader()
// + ".\n The classloader of the test class is "
// + extensionContext.getRequiredTestClass().getClassLoader());

return true;
}
Expand Down
Loading

0 comments on commit e05fa42

Please sign in to comment.