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

Java 10: "Illegal method name" when test functions in Kotlin contain spaces in name [SPR-17137] #21674

Closed
spring-issuemaster opened this issue Aug 7, 2018 · 15 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Aug 7, 2018

Ömer Yildiz opened SPR-17137 and commented

// Kotlin code:

@RunWith(SpringRunner::class)
@JooqTest
class Test {    

    fun `this will fail`() {  }

}

 

Running this Kotlin test in a java 9 or 10 environment throws the following error:

org.springframework.aop.framework.AopConfigException: Could not generate CGLIB subclass of class [class Test]: Common causes of this problem include using a final class or a non-visible class; nested exception is org.springframework.cglib.core.CodeGenerationException: java.lang.reflect.InvocationTargetException-->null	at org.springframework.aop.framework.CglibAopProxy.getProxy(CglibAopProxy.java:209)
	at org.springframework.aop.framework.ProxyFactory.getProxy(ProxyFactory.java:110)
	at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.createProxy(AbstractAutoProxyCreator.java:473)
	at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.wrapIfNecessary(AbstractAutoProxyCreator.java:355)
	at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.postProcessAfterInitialization(AbstractAutoProxyCreator.java:304)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsAfterInitialization(AbstractAutowireCapableBeanFactory.java:437)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1706)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:413)
	at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.injectDependencies(DependencyInjectionTestExecutionListener.java:119)
	at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.prepareTestInstance(DependencyInjectionTestExecutionListener.java:83)
	at org.springframework.boot.test.autoconfigure.SpringBootDependencyInjectionTestExecutionListener.prepareTestInstance(SpringBootDependencyInjectionTestExecutionListener.java:44)
	at org.springframework.test.context.TestContextManager.prepareTestInstance(TestContextManager.java:246)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.createTest(SpringJUnit4ClassRunner.java:227)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner$1.runReflectiveCall(SpringJUnit4ClassRunner.java:289)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.methodBlock(SpringJUnit4ClassRunner.java:291)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:246)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:97)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:190)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: org.springframework.cglib.core.CodeGenerationException: java.lang.reflect.InvocationTargetException-->null
	at org.springframework.cglib.core.AbstractClassGenerator.generate(AbstractClassGenerator.java:345)
	at org.springframework.cglib.proxy.Enhancer.generate(Enhancer.java:492)
	at org.springframework.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:93)
	at org.springframework.cglib.core.AbstractClassGenerator$ClassLoaderData$3.apply(AbstractClassGenerator.java:91)
	at org.springframework.cglib.core.internal.LoadingCache$2.call(LoadingCache.java:54)
	at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java)
	at org.springframework.cglib.core.internal.LoadingCache.createEntry(LoadingCache.java:61)
	at org.springframework.cglib.core.internal.LoadingCache.get(LoadingCache.java:34)
	at org.springframework.cglib.core.AbstractClassGenerator$ClassLoaderData.get(AbstractClassGenerator.java:116)
	at org.springframework.cglib.core.AbstractClassGenerator.create(AbstractClassGenerator.java:291)
	at org.springframework.cglib.proxy.Enhancer.createHelper(Enhancer.java:480)
	at org.springframework.cglib.proxy.Enhancer.createClass(Enhancer.java:337)
	at org.springframework.aop.framework.ObjenesisCglibAopProxy.createProxyClassAndInstance(ObjenesisCglibAopProxy.java:58)
	at org.springframework.aop.framework.CglibAopProxy.getProxy(CglibAopProxy.java:205)
	... 31 more
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.GeneratedMethodAccessor43.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.springframework.cglib.core.ReflectUtils.defineClass(ReflectUtils.java:459)
	at org.springframework.cglib.core.AbstractClassGenerator.generate(AbstractClassGenerator.java:336)
	... 45 more
Caused by: java.lang.ClassFormatError: Illegal method name "this will fail" in class Test$$EnhancerBySpringCGLIB$$9a76c928
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1007)
	... 50 more

 

It doesn't matter if @JooqTest or @DataJpaTest is used. As soon as Spring attempts to proxy (with the transaction handler) the test class it fails.


Affects: 5.0.5

Issue Links:

  • #14113 Test instances should not be proxied in the TestContext framework
  • #21739 Revisit @Bean introspection between @Configuration classes and 'lite' beans

3 votes, 9 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 7, 2018

Juergen Hoeller commented

A general note: Such latest-infrastructure test arrangements should always be run against the latest Spring Framework version; 5.0.5 seems rather old here. I'm not aware of any particular differences in this area in 5.0.8, but 5.1 RC1 has a revised CGLIB class definition mechanism which might make a difference for this case.

That said, if the issue is reproducible against 5.1 RC1 / 5.1.0.BUILD-SNAPSHOT, we'll have to see how Kotlin handles spaces in method names, possibly adopting the same convention at CGLIB level so that the JVM accepts those names even for generated proxy methods.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 7, 2018

Ömer Yildiz commented

In order to check it, I've updated my parent's pom to use spring boot 2.1.0.BUILD-SNAPSHOT (which imports spring 5.1 RC1).

However, the problem still persists.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2018

Benjamin Winterberg commented

I'm encountering the exact same problem while migrating our product to JDK 10. We use Spring Framework 5.0.8 (no boot), JUnit 5, Kotlin 1.2.60 and cglib-nodeps 3.2.7.

Normal method names (without backticks) work as expected but Kotlin backtick method names with spaces result in the exception above. As a workaround you could rename test methods to not use backticks.

However we have nearly 1.000 Kotlin Spring tests with a bunch of test names containing special characters such as spaces. It would be quite a lot of work to rename all those tests.

Any ETA when you'll work on this issue? Also please let me know if you need more info to reproduce.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2018

Juergen Hoeller commented

Looking at that stacktrace, I'm wondering why there is even a CGLIB proxy generated for the test class in the first place? DependencyInjectionTestExecutionListener is going to ignore a potential proxy in any case (see how it ignores the result from the beanFactory.initializeBean call), simply proceeding with the original instance...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2018

Sébastien Deleuze commented

This can be reproduced on https://github.com/spring-guides/tut-spring-boot-kotlin and this is indeed something we need to fix before 5.1 GA. The issue occurs with Boot 2.0 / Framework 5.0 as well so it is likely not a 5.1 regression.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2018

Juergen Hoeller commented

If for any reason we can't fully support spaces in method names for generated proxies quite yet, we should at least avoid the proxy attempt there in the first place. The AOP setup apparently reacts to some general advisor here, trying to proxy the test class itself with it... which is entirely pointless as far as I can tell.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 20, 2018

Davin Kevin commented

FYI, this problem doesn't happen on a simple test, without database (like rest endpoint or business logic). The operation made by @JooqTest or @DataJpaTest with transaction seems to be the cause of the problem

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 20, 2018

Juergen Hoeller commented

Hmm, I suppose Spring's transaction advisor picks up the test instance there, detects implicit transaction annotations on it (as meta-annotations on those stereotype annotations) and therefore tries to proxy it... with the proxy subsequently ignored but nevertheless causing a failure here. This doesn't make sense at all: We need to figure out how to suppress the proxy attempt there to begin with.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 22, 2018

Juergen Hoeller commented

I've introduced an "original instance" convention to AutowireCapableBeanFactory that DependencyInjectionTestExecutionListener uses now, with Spring AOP's auto-proxy creators consistently ignoring such beans by name convention now, even if some advisors would (accidentally) match. As a consequence, the test context framework should not attempt to create a proxy for such a test instance anymore, bypassing the problem with illegal method names to begin with.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 22, 2018

Juergen Hoeller commented

Reopened since a few integration tests need to be revisited still...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 23, 2018

Juergen Hoeller commented

This is finally in master now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 24, 2018

Marten Deinum commented

I also noticed some changes in regards to the Test Context Framework. Would this then fix #14113 and/or #9309 (which was a proposed solution for #14113) as well? 

 

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 24, 2018

Sam Brannen commented

Marten, hold that thought!

I promise to get back to you on that. ;-)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 24, 2018

Juergen Hoeller commented

As far as I can see right now, this should completely address #14113 indeed, whereas #9309 is somewhat different in its goals.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 24, 2018

Sam Brannen commented

I have marked #14113 as resolved for Spring Framework 5.1 RC3 (and reassigned it to Juergen).

Whereas, #9309 remains open in the General Backlog for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.