Skip to content
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.

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

@MethodSource fails to load class with custom ClassLoader arrangement #3279

Closed
chrisr3 opened this issue May 1, 2023 · 15 comments
Closed

@MethodSource fails to load class with custom ClassLoader arrangement #3279

chrisr3 opened this issue May 1, 2023 · 15 comments

Comments

@chrisr3
Copy link
Contributor

chrisr3 commented May 1, 2023

JUnit tests with @MethodSource fail inside an OSGi framework after upgrading from 5.9.2 to 5.9.3.

# Execution Started: testMethodSource(String) - [engine:bnd-bundle-engine]/[bundle:test-junit5-tests;1.0.0.SNAPSHOT]/[sub-engine:junit-jupiter]/[class:org.testing.junit.TestJUnit]/[test-template:testMethodSource(java.lang.String)]
# Execution Finished: testMethodSource(String) - [engine:bnd-bundle-engine]/[bundle:test-junit5-tests;1.0.0.SNAPSHOT]/[sub-engine:junit-jupiter]/[class:org.testing.junit.TestJUnit]/[test-template:testMethodSource(java.lang.String)] - TestExecutionResult [status = FAILED, throwable = org.junit.platform.commons.JUnitException: Could not load class [org.testing.junit.TestJUnit]]
org.junit.platform.commons.JUnitException: Could not load class [org.testing.junit.TestJUnit]
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.lambda$loadRequiredClass$8(MethodArgumentsProvider.java:181)
        at org.junit.platform.commons.function.Try$Failure.getOrThrow(Try.java:335)
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.loadRequiredClass(MethodArgumentsProvider.java:180)
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.findFactoryMethodByFullyQualifiedName(MethodArgumentsProvider.java:117)
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.findFactoryMethod(MethodArgumentsProvider.java:82)
        ...
Caused by: java.lang.ClassNotFoundException: org.testing.junit.TestJUnit
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
        at org.junit.platform.commons.util.ReflectionUtils.lambda$tryToLoadClass$9(ReflectionUtils.java:831)
        at org.junit.platform.commons.function.Try.lambda$call$0(Try.java:57)
        at org.junit.platform.commons.function.Try.of(Try.java:93)
        ...

Steps to reproduce

Run attached Gradle project:

$ ./gradlew clean build

Context

  • Used versions (Jupiter/Vintage/Platform):
    Jupiter 5.9.3
    Platform 1.9.3
  • Build Tool/IDE:
    Gradle 8.1.1
    Java 11
@marcphilipp
Copy link
Member

Could be a red herring but there's also a suppressed exception that could be related to #3131 or #3266:

org.junit.platform.commons.JUnitException: Could not load class [org.testing.junit.TestJUnit]
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.lambda$loadRequiredClass$8(MethodArgumentsProvider.java:181)
        at org.junit.platform.commons.function.Try$Failure.getOrThrow(Try.java:335)
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.loadRequiredClass(MethodArgumentsProvider.java:180)
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.findFactoryMethodByFullyQualifiedName(MethodArgumentsProvider.java:117)
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.findFactoryMethod(MethodArgumentsProvider.java:82)
        at org.junit.jupiter.params.provider.MethodArgumentsProvider.lambda$provideArguments$1(MethodArgumentsProvider.java:59)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:274)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:274)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:110)
        at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:44)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
        at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
        at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
        at aQute.tester.bundle.engine.BundleDescriptor.executeChild(BundleDescriptor.java:49)
        at aQute.tester.bundle.engine.BundleEngine.lambda$executeBundle$7(BundleEngine.java:120)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
        at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
        at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at aQute.tester.bundle.engine.BundleEngine.executeBundle(BundleEngine.java:120)
        at aQute.tester.bundle.engine.BundleEngine.lambda$execute$5(BundleEngine.java:100)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
        at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
        at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
        at aQute.tester.bundle.engine.BundleEngine.execute(BundleEngine.java:100)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
        at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
        at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
        at aQute.tester.junit.platform.Activator.test(Activator.java:441)
        at aQute.tester.junit.platform.Activator.automatic(Activator.java:345)
        at aQute.tester.junit.platform.Activator.run(Activator.java:217)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
        at aQute.launcher.Launcher.handleMainCallable(Launcher.java:465)
        at aQute.launcher.Launcher.launch(Launcher.java:438)
        at aQute.launcher.Launcher.run(Launcher.java:186)
        at aQute.launcher.Launcher.main(Launcher.java:162)
        at aQute.launcher.pre.EmbeddedLauncher.executeWithRunPath(EmbeddedLauncher.java:170)
        at aQute.launcher.pre.EmbeddedLauncher.findAndExecute(EmbeddedLauncher.java:135)
        at aQute.launcher.pre.EmbeddedLauncher.main(EmbeddedLauncher.java:52)
        Suppressed: org.junit.platform.commons.PreconditionViolationException: Configuration error: You must configure at least one set of arguments for this @ParameterizedTest
                at org.junit.platform.commons.util.Preconditions.condition(Preconditions.java:299)
                at org.junit.jupiter.params.ParameterizedTestExtension.lambda$provideTestTemplateInvocationContexts$5(ParameterizedTestExtension.java:98)
                at java.base/java.util.stream.AbstractPipeline.close(AbstractPipeline.java:323)
                at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:271)
                ... 87 more
Caused by: java.lang.ClassNotFoundException: org.testing.junit.TestJUnit
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
        at org.junit.platform.commons.util.ReflectionUtils.lambda$tryToLoadClass$9(ReflectionUtils.java:831)
        at org.junit.platform.commons.function.Try.lambda$call$0(Try.java:57)
        at org.junit.platform.commons.function.Try.of(Try.java:93)
        at org.junit.platform.commons.function.Try.call(Try.java:57)
        at org.junit.platform.commons.util.ReflectionUtils.tryToLoadClass(ReflectionUtils.java:794)
        at org.junit.platform.commons.util.ReflectionUtils.tryToLoadClass(ReflectionUtils.java:750)
        ... 111 more

@marcphilipp
Copy link
Member

There were definitely some changes to MethodArgumentsProvider in 5.9.3 but I suspect the root cause for the class-loading issue is in bnd-bundle-engine.

@chrisr3 Could you please raise an issue with the BND folks and cross-reference it here?

@chrisr3
Copy link
Contributor Author

chrisr3 commented May 1, 2023

Done.

My first impression after examining the exception stack trace was that JUnit was (suddenly?) expecting the test class to be found in the Application classloader:

Caused by: java.lang.ClassNotFoundException: org.testing.junit.TestJUnit
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
        at org.junit.platform.commons.util.ReflectionUtils.lambda$tryToLoadClass$9(ReflectionUtils.java:831)
        ...

I would consider this to be unlikely when executed within an OSGi framework.


According to git bisect:

07f2bf349e45f5a63228cb6002ae7bdbd3445ce1 is the first bad commit
commit 07f2bf349e45f5a63228cb6002ae7bdbd3445ce1
Author: Sam Brannen <sam@sambrannen.com>
Date:   Sat Apr 22 12:20:31 2023 +0200

    Treat overloaded local & external @MethodSource factory methods equally

@chrisr3
Copy link
Contributor Author

chrisr3 commented May 2, 2023

FWIW this small change fixes my test-case:

diff --git a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java
index ddcb16b5c4..ba3d9d4874 100644
--- a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java
+++ b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java
@@ -108,7 +108,7 @@ class MethodArgumentsProvider extends AnnotationBasedArgumentsProvider<MethodSou
                String className = methodParts[0];
                String methodName = methodParts[1];
                String methodParameters = methodParts[2];
-               Class<?> clazz = loadRequiredClass(className);
+               Class<?> clazz = testMethod.getDeclaringClass();
 
                // Attempt to find an exact match first.
                Method factoryMethod = ReflectionUtils.findMethod(clazz, methodName, methodParameters).orElse(null);

The problem is that loadRequiredClass(className) uses either the TCCL or the Application classloader, neither of which is likely to be able to load the test class within an OSGi framework.

If testMethod.getDeclaringClass() isn't what you want either then maybe:

Class<?> clazz = loadRequiredClass(className, testMethod.getDeclaringClass().getClassLoader());

where

private static Class<?> loadRequiredClass(String className, ClassLoader classLoader) {
    return ReflectionUtils.tryToLoadClass(className, classLoader).getOrThrow(
        cause -> new JUnitException(format("Could not load class [%s]", className), cause));
}

will work instead?

@marcphilipp
Copy link
Member

With 5.9.2, putting the getArgs() method as a static method into another class and referencing it via @MethodSource("com.acme.OtherClass#getArgs") also fails with a ClassNotFoundException. I think following your proposal makes sense but should be applied to all call sites of ReflectionUtils.tryToLoadClass in Jupiter where a test class is already available. A quick search yielded MethodBasedCondition and StringToCommonJavaTypesConverter.

@chrisr3 Would you have time to submit a PR for that against main?

@chrisr3
Copy link
Contributor Author

chrisr3 commented May 2, 2023

Sorry, I am unable to build JUnit because I do not have "Java Flight Recorder", and have no idea where to get it from.

I was only able to bisect by deleting the junit-platform-jfr/src/main/java/org/junit/platform/jfr/*.java classes and disabling all of tests!

@sormuras
Copy link
Member

sormuras commented May 2, 2023

Sorry, I am unable to build JUnit because I do not have "Java Flight Recorder", and have no idea where to get it from.

Curious, which distribution of JDK 17 do you use?

@chrisr3
Copy link
Contributor Author

chrisr3 commented May 2, 2023

Curious, which distribution of JDK 17 do you use?

$ java -version
openjdk version "17.0.6" 2023-01-17
OpenJDK Runtime Environment (Red_Hat-17.0.6.0.10-1.fc37) (build 17.0.6+10)
OpenJDK 64-Bit Server VM (Red_Hat-17.0.6.0.10-1.fc37) (build 17.0.6+10, mixed mode, sharing)

@kriegfrj
Copy link
Contributor

kriegfrj commented May 3, 2023

With 5.9.2, putting the getArgs() method as a static method into another class and referencing it via @MethodSource("com.acme.OtherClass#getArgs") also fails with a ClassNotFoundException. I think following your proposal makes sense but should be applied to all call sites of ReflectionUtils.tryToLoadClass in Jupiter where a test class is already available. A quick search yielded MethodBasedCondition and StringToCommonJavaTypesConverter.

@chrisr3 Would you have time to submit a PR for that against main?

For reference: #1987, #2104, #2106. Both of these issues were resolved without actually being completed, because they were going to be addressed in #806, but unfortunately #806 is still pending.

My proposed solution in #2106 is essentially the same as those that @chrisr3 is suggesting here.

In general, I think that using the test class' class loader would be the best solution for these types of problems. In the OSGi context, this puts the power back in the hands of the test developer because they can control which classes/packages are visible to the test class (and hence to the test class' classloader). In the case of (eg) a @MethodSource("com.acme.OtherClass#getArgs") reference, you simply need to make sure that your test class' bundle imports the package com.acme and (if we're using the test class' classloader) it then it will load.

If I have a look at this in Bnd I might be able to find a workaround, as I have for the other issues (like #2104). But my workarounds essentially involve the Bnd tester doing more and more of the class loading on JUnit's behalf in order to overcome these issues, and I think I've probably picked all the low-hanging fruit - the issues are becoming increasingly difficult to work around as they are being found increasingly deeper inside JUnit. It would be nicer and cleaner if these issues were resolved in JUnit.

@chrisr3
Copy link
Contributor Author

chrisr3 commented May 3, 2023

Curious, which distribution of JDK 17 do you use?

Aha! I have found Java Flight Recorder in this package:

java-17-openjdk-jmods-17.0.6.0.10-1.fc37.x86_64

I had assumed that installing the package containing the pre-compiled shared objects was enough:

java-17-openjdk-headless-17.0.6.0.10-1.fc37.x86_64

As indeed it appears to be, for everything except Java Flight Recorder.


See #3280.

@kriegfrj
Copy link
Contributor

kriegfrj commented May 3, 2023

My proposed solution in #2106 is essentially the same as those that @chrisr3 is suggesting here.

I realise that I should have been a bit more specific.

I think that this:

-               Class<?> clazz = loadRequiredClass(className);
+               Class<?> clazz = testMethod.getDeclaringClass();

...is the best solution. There's no point loading the class when it's already been loaded.

@chrisr3
Copy link
Contributor Author

chrisr3 commented May 3, 2023

There's no point loading the class when it's already been loaded.

Heh, I did wonder about that 😉. But it also occurred to me that the method might exist on a parent class.

@marcphilipp
Copy link
Member

That solution would break using a static method in another class, though. So I think using the test class' class loader to load the class would work in both cases and would be a no-op (since class loaders cache classes that have already been loaded) when className equals the test class.

@kriegfrj
Copy link
Contributor

kriegfrj commented May 3, 2023

But it also occurred to me that the method might exist on a parent class.

I don't think that will matter. The parent class will be visible to the subclass.

That solution would break using a static method in another class, though. So I think using the test class' class loader to load the class would work in both cases and would be a no-op (since class loaders cache classes that have already been loaded) when className equals the test class.

Yes, you're right. So @chrisr3 's second implementation would work for both cases. The overhead in "reloading" the already-loaded test class for unqualified method names is small and is justified by the simplification it makes to the code.

Perhaps then we might need a usage note for OSGi users somewhere, to make sure that if you're using a fully-qualified reference to a static method in another class, your test bundle must import the package that the class lives in.

@marcphilipp marcphilipp linked a pull request May 3, 2023 that will close this issue
@sbrannen sbrannen changed the title JUnit 5.9.3 MethodSource fails inside OSGi framework @MethodSource fails to load class inside OSGi framework May 4, 2023
@sbrannen sbrannen changed the title @MethodSource fails to load class inside OSGi framework @MethodSource fails to load class with custom ClassLoader arrangement May 9, 2023
@sbrannen sbrannen self-assigned this May 12, 2023
sbrannen pushed a commit to sbrannen/junit5 that referenced this issue May 12, 2023
Prior to this commit, the extensions supporting @MethodSource,
@EnabledIf, and @DisabledIf failed to load an external class if the
external class was not visible to JUnit's default ClassLoader.

This commit addresses those issues by loading external classes using
the ClassLoader obtained from the test class or falling back to JUnit's
default ClassLoader if the test's ClassLoader cannot be obtained.

This allows JUnit to find the classes while running in a custom
ClassLoader arrangement -- for example, inside an OSGi framework.

Closes junit-team#3292
Closes junit-team#3279
Closes junit-team#3280
sbrannen added a commit to sbrannen/junit5 that referenced this issue May 12, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue May 12, 2023
@sbrannen

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants