Skip to content

Avoid the need to preload mock Kubernetes server classes for Kubernetes tests to pass #48977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 18, 2025
Merged

Avoid the need to preload mock Kubernetes server classes for Kubernetes tests to pass #48977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions .../src/main/java/io/quarkus/kubernetes/client/deployment/NoQuarkusTestKubernetesClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.quarkus.kubernetes.client.deployment;

import java.util.Arrays;
import java.util.function.BooleanSupplier;

/**
Expand All @@ -13,13 +12,22 @@
* to avoid starting a Kubernetes test container in such a case.
*/
class NoQuarkusTestKubernetesClient implements BooleanSupplier {
static final String IO_QUARKUS_TEST_KUBERNETES_CLIENT_PACKAGE = "io.quarkus.test.kubernetes.client";
static final Boolean IO_QUARKUS_TEST_KUBERNETES_CLIENT_AVAILABLE = Arrays.stream(Package.getPackages())
.map(Package::getName)
.anyMatch(p -> p.startsWith(IO_QUARKUS_TEST_KUBERNETES_CLIENT_PACKAGE));
static final String IO_QUARKUS_TEST_KUBERNETES_CLIENT_PACKAGE_CLASS = "io.quarkus.test.kubernetes.client.AbstractKubernetesTestResource";
// We cannot assume what order classes are loaded in, so check if a known class can be loaded rather than looking at what's already loaded
static final boolean IO_QUARKUS_TEST_KUBERNETES_CLIENT_AVAILABLE = isClassAvailable(
IO_QUARKUS_TEST_KUBERNETES_CLIENT_PACKAGE_CLASS);

@Override
public boolean getAsBoolean() {
return !IO_QUARKUS_TEST_KUBERNETES_CLIENT_AVAILABLE;
}

private static boolean isClassAvailable(String className) {
try {
Class.forName(className);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is correct, instead of the the variant where we pass the TCCL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered about that. The example I coped did in fact use the TCCL, but I decided the TCCL wasn't the right choice, for two reasons. The first is that it would just go back through the FacadeClassLoader, and the FCL is a kind of expensive way to load classes. (It loads each class twice, and does inspection of the class to decide how to load it.) Because the class we're looking for is a Quarkus class, I decided that the classloader used to load the kube extension would also have good access to the test framework libraries, and could safely load it.
The second reason I didn't use the TCCL is that I wasn't 100% sure what it would be at that point, and whether it would be the FCL or a runtime classloader, and I've been fighting so many (self-inflicted) "TCCL is wrong at this point" chaos-bugs I was nervous to increase our reliance on the TCCL. :)

I think it probably actually doesn't matter, and all three options (FCL, defining classloader, or runtime classloader) would end up delegating to the system classloader. So if you think TCCL is a safer/less surprising choice, happy to change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also read the docs (which, admittedly, I should have done first, rather than going with intuition), and Package.getPackages() uses the caller's classloader, and Class.forName also uses the caller's classloader. So I've kept parity.

Class.forName does always initialise the class, which could be a waste if it wasn't used, but I think if it's on the classpath it's likely to be used (because if it's not used, the Kubernetes tests will probably fail :) )

return true;
} catch (ClassNotFoundException e) {
return false;
}
}
}
45 changes: 3 additions & 42 deletions ...-framework/junit5/src/main/java/io/quarkus/test/junit/classloading/FacadeClassLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,6 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
}

if (isQuarkusTest && !isIntegrationTest) {

preloadTestResourceClasses(inspectionClass);
QuarkusClassLoader runtimeClassLoader = getQuarkusClassLoader(inspectionClass, profile);
Class<?> clazz = runtimeClassLoader.loadClass(name);

Expand All @@ -334,14 +332,13 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
}

} catch (NoSuchMethodException e) {
// TODO better handling of these
System.err.println("Could not get method " + e);
log.error("Could not get method " + e);
throw new RuntimeException(e);
} catch (InvocationTargetException e) {
System.err.println("Could not invoke " + e);
log.error("Could not invoke " + e);
throw new RuntimeException(e);
} catch (IllegalAccessException e) {
System.err.println("Could not access " + e);
log.error("Could not access " + e);
throw new RuntimeException(e);
}

Expand Down Expand Up @@ -378,42 +375,6 @@ private boolean registersQuarkusTestExtensionWithExtendsWith(Class<?> inspection

}

/*
* What's this for?
* It's a bit like detecting the location in a privacy test or detecting the lab environment in an emissions test and then
* deciding how to behave.
* We're special-casing behaviour for a hard-coded selection of test packages. Yuck!
* TODO Hopefully, once https://github.com/quarkusio/quarkus/issues/45785 is done, it will not be needed.
* Some tests, especially in kubernetes-client and openshift-client, check config to decide whether to start a dev service.
* That happens at augmentation, which happens before test execution.
* In the old model, the test class would have already been loaded by JUnit first, and it would have had a chance to write
* config to the system properties.
* That config would influence whether dev services were started.
* TODO even without 45785 it might be nice to find a better way, perhaps rewriting the AbstractKubernetesTestResource test
* resource to work differently?
*
*/
private void preloadTestResourceClasses(Class<?> fromCanary) {
try {
Class<Annotation> ca = (Class<Annotation>) peekingClassLoader
.loadClass("io.quarkus.test.common.QuarkusTestResource");
List<Annotation> ans = AnnotationSupport.findRepeatableAnnotations(fromCanary, ca);
for (Annotation a : ans) {
Method m = a
.getClass()
.getMethod(VALUE);
Class<?> resourceClass = (Class<?>) m.invoke(a);
// Only do this hack for the resources we know need it, since it can cause failures in other areas
if (resourceClass.getName().contains("Kubernetes")) {
getParent().loadClass(resourceClass.getName());
}
}
} catch (ClassNotFoundException | InvocationTargetException | NoSuchMethodException | IllegalAccessException e) {
// In some projects, these classes are not on the canary classpath. That's fine, we know there's nothing to preload.
log.debug("Canary classloader could not preload test resources:" + e);
}
}

private boolean registersQuarkusTestExtensionOnField(Class<?> inspectionClass) {

try {
Expand Down
Loading