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

Annotation classloading issue in custom JUnit extension when run in continuous testing #24257

Open
famod opened this issue Mar 10, 2022 · 20 comments
Labels

Comments

@famod
Copy link
Member

famod commented Mar 10, 2022

Describe the bug

A Junit extension, e.g. a org.junit.jupiter.api.extension.BeforeEachCallback, that is registered via src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension, cannot find a custom annotation that is present on a test method.

context.getRequiredTestMethod().getAnnotationsByType(SomeAnnotation.class) (context being org.junit.jupiter.api.extension.ExtensionContext) returns an empty array.
OTOH, context.getRequiredTestMethod().getAnnotations() does contain the annotation. So it's there, but the type is loaded in two different classloaders:

Via surefire (where everything is ok):

::: CL1: jdk.internal.loader.ClassLoaders$AppClassLoader@277050dc
::: CL2: jdk.internal.loader.ClassLoaders$AppClassLoader@277050dc

(CL1 is SomeAnnotation.class.getClassLoader() and CL2 is annotation.annotationType().getClassLoader())

But in CT:

::: CL1: QuarkusClassLoader:Augmentation Class Loader: TEST@7497f2dc
::: CL2: QuarkusClassLoader:Deployment Class Loader: TEST@61ad719b

ℹ️ There is no issue when the extension is registered via @ExtendWith(...)!

Expected behavior

Annotation is found in CT just like when run via surefire.

Actual behavior

Annotation is not found by type because two different classloaders are involved.

How to Reproduce?

q_ct-callback-ann_multi.zip

  • mvn clean install ✔️
  • mvn quarkus:test -f dist

Note that this reproducer needs two modules because of #24229 and in reality, core produces a test-jar but I wanted to take that out of the equation.

Edit: Udpated reproducer is here: #24257 (comment)

Output of uname -a or ver

No response

Output of java -version

17.0.2, vendor: BellSoft

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.7.3.Final and 2.7.4.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.4

Additional information

Reminds me a lot of #10623, but that was long before CT was invented.

@famod famod added the kind/bug Something isn't working label Mar 10, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 10, 2022

/cc @stuartwdouglas

@aloubyansky
Copy link
Member

@famod this appears to be fixed in main (but fails in 2.7.5.Final). I suppose #24440 fixed it.
I did see your comment #24229 (comment), are you sure it's still an issue?

@famod
Copy link
Member Author

famod commented Mar 23, 2022

@aloubyansky well, I checked it with my real project while trying out #24440 and it wasn't fixed.

I'll try again with current main. Maybe it's now fixed for the reproducer but not my real project (I hope not).

@aloubyansky
Copy link
Member

Please do and see if you can adjust the reproducer for main. Thanks!

@famod
Copy link
Member Author

famod commented Mar 23, 2022

I can confirm that it's fixed for the reproducer, but not for my real project. I'm somehow suspecting test-jar, will try to reproduce.

@famod
Copy link
Member Author

famod commented Mar 23, 2022

It's not test-jar (at least it works in the small reproducer). Great, once again I have to find the needle in the haystack...

@famod
Copy link
Member Author

famod commented Mar 23, 2022

Interesting: In my project where it fails (it doesn't fail everywhere!) there are two different deployment classloaders involved!
In cases where it's fine it's the same deployment CL, just like in the simple reproducer.

Will try to dig deeper tomorrow, but in case you have an idea @aloubyansky please let me know! 🙂

@famod
Copy link
Member Author

famod commented Mar 23, 2022

One more detail: Right before the test failure there is a @QuarkusTest (with a @QuarkusTestResource, hence triggering a Quarkus restart) and the test that's failing right after is a non-Quarkus test!

In that @QuarkusTest the annotation loading is fine.

@famod
Copy link
Member Author

famod commented Mar 23, 2022

Managed to reproduce it, it was just a matter of adding a non-Quarkus test: q_ct-callback-ann_multi-2.zip

@famod
Copy link
Member Author

famod commented Mar 24, 2022

I suppose the deployment classloaders mismatch is caused by the creation of a new deployment classloader during discovery of unit test classes:

cl = testApplication.createDeploymentClassLoader();
cl.reset(Collections.emptyMap(), transformedClasses);
for (String i : unitTestClasses) {
try {
utClasses.add(cl.loadClass(i));

/cc @stuartwdouglas @aloubyansky

@famod
Copy link
Member Author

famod commented Mar 24, 2022

I think I have a fix for this, stay tuned!

@famod
Copy link
Member Author

famod commented Mar 24, 2022

@stuartwdouglas @aloubyansky
I'm kinda stuck. This is what I came up with: main...famod:ct-unit-transform-cl
Which does resolves this issue, but it somehow also partially breaks change detection for QuarkusTest, meaning a QuarkusTest will no re-run if a class it depends on changes in an upstream module.
After adding a beans.xml to the upstream module the QuarkusTest is fine, but the unit test is not reloaded for upstream changes...

I'm under the impression that unit tests must be run separately, with a separate TestsPlan, with that special deployment classloader as TCCL...?

@famod
Copy link
Member Author

famod commented Mar 24, 2022

PS: I noticed that reloading of unit tests is partially broken the same way, even in 2.7.5.Final (without my change).
I think that was due to a missing beans.xml...

@aloubyansky
Copy link
Member

@famod just tried your last reproducer from #24257 (comment). It fails with 2.7.5.Final but passes with 2.8.0.Final and the main branch.

@famod
Copy link
Member Author

famod commented Apr 8, 2022

@aloubyansky hum, still fails for me with 2.8.0.Final and main (on two different machines):

mvn clean install && mvn quarkus:test -f dist

@aloubyansky
Copy link
Member

Right. Not sure what happened. Just tried on another laptop and it fails. Sorry.

@famod
Copy link
Member Author

famod commented Apr 8, 2022

No worries, thanks for checking!

@holly-cummins
Copy link
Contributor

I wonder if this is also related to #27821?

@famod
Copy link
Member Author

famod commented Jan 16, 2023

@holly-cummins thanks for the pointer! At first glance #27821 seems to be about testing in general, not just continuous testing, right?
Thing is that this issue here only manifests in CT.

@holly-cummins
Copy link
Contributor

holly-cummins commented Jan 16, 2023

Oh, good point. Most of my pain is related to continuous testing, so I assumed #27821 also was, but the details in #27821 says it affects both modes. Good job I made that note. :)

So it will be distantly related, if it's related.

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

No branches or pull requests

3 participants