search

quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.87k stars 2.71k forks source link

Broken QuarkusTestExtension #8446

Open alesj opened 4 years ago

alesj commented 4 years ago

When using Quarkus 1.3.1.Final or current master against our Registry, I get this:

Zulip thread:

As mentioned / described, we use custom JUnit5 extension

To reproduce, simply change Quarkus version here:

alesj commented 4 years ago

newMethod = {java.lang.reflect.Method@8575} "void io.apicurio.registry.ArtifactStateTest.testEnableDisableArtifact(io.apicurio.registry.client.RegistryService)" clazz = {java.lang.Class@4485} "class io.apicurio.registry.ArtifactStateTest" slot = 5 name = "testEnableDisableArtifact" returnType = {java.lang.Class@8581} "void" parameterTypes = {java.lang.Class[1]@8582} 0 = {java.lang.Class@8475} "interface io.apicurio.registry.client.RegistryService" cachedConstructor = null newInstanceCallerCache = null name = "io.apicurio.registry.client.RegistryService" classLoader = {io.quarkus.bootstrap.classloading.QuarkusClassLoader@8592} "QuarkusClassLoader:Quarkus Base Runtime ClassLoader"

Where actual argument then gets this classloader ...

argumentsFromTccl = {java.util.ArrayList@8577} size = 1 0 = {io.apicurio.registry.client.CachedRegistryService@8595} "" getClass().getClassLoader() = {sun.misc.Launcher$AppClassLoader@6990}

geoand commented 4 years ago

I went through this and unfortunately there is no good solution...

Although the code in the custom extension can be hacked to load everything from the TCCL instead of the CL which the methods run, that won't cut it unfortunately because JUnit then validates the returned object against the parameter type (and of course this validation fails because of the different ClassLoaders).

This is somewhat related to #8252.

Perhaps your custom extension could return a proxy (constructed on the regular CL) that then delegates the calls to the real object which is constructed on the TCCL.

alesj commented 4 years ago

Perhaps your custom extension could return a proxy (constructed on the regular CL) that then delegates the calls to the real object which is constructed on the TCCL.

How would this help? Wouldn't this still create a mismatch?

geoand commented 4 years ago

Ah sorry, yes you are right....

gsmet commented 4 years ago

/cc @stuartwdouglas

alesj commented 4 years ago

I worked around this by introducing Supplier

geoand commented 4 years ago

You did need to make the changes I suggested in chat, right?

alesj commented 4 years ago

Which changes?

I did need to use reflection yes.

See PR.

On Wed, 8 Apr 2020 at 19:51, Georgios Andrianakis notifications@github.com wrote:

You did need to make the changes I suggested in chat, right?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/quarkusio/quarkus/issues/8446#issuecomment-611100069, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACRA6G3ROIF2CPXXVYT7GLRLS2QRANCNFSM4MDCW7HQ .

geoand commented 4 years ago

Yeah, that's what I meant. The use of reflection in your extension

On Wed, Apr 8, 2020, 21:24 Aleš Justin notifications@github.com wrote:

Which changes?

I did need to use reflection yes.

See PR.

On Wed, 8 Apr 2020 at 19:51, Georgios Andrianakis < notifications@github.com> wrote:

You did need to make the changes I suggested in chat, right?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://github.com/quarkusio/quarkus/issues/8446#issuecomment-611100069 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACRA6G3ROIF2CPXXVYT7GLRLS2QRANCNFSM4MDCW7HQ

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quarkusio/quarkus/issues/8446#issuecomment-611116525, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBMDP3SCJ5T2FF7TTXP4G3RLS6NVANCNFSM4MDCW7HQ .

stuartwdouglas commented 4 years ago

I dont think there is much we can do here without JUnit changes. They are considering adding better support for custom ClassLoader implementations in the next release, so hopefully that happens in a way that is useful to us.

geoand commented 2 months ago

This is one that would potentially be fixed by @holly-cummins's WIP