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

Ref #4894: Make Groovy DSL ITs platform compliant #5035

Merged

Conversation

essobedo
Copy link
Contributor

fixes #4894

Motivation

The Groovy DSL tests are hard to make work on the Quarkus Platform.

Modifications

  • Revert changes that were required to workaround the ClassLoading issue

@essobedo
Copy link
Contributor Author

@ppalaga I've just checked and refreshed my mind a bit about it, the problem here is very specific and complex, it only occurs when using the Quarkus JUnit5 extension with Groovy extensions. The Groovy extensions are loaded with the System ClassLoader instead of the Quarkus Runtime ClassLoader because Groovy is declared as a parent first artifact in Quarkus core that is why the Camel extensions are not seen. Groovy has been declared as a parent's first artifact to fix a ClassValue leak in Quarkus described in this ticket. In theory, the way to fix the leak is more or less what is done in the GroovyCacheCleaner in case Groovy is in the System CL but if Groovy is in a custom CL like Quarkus Runtime ClassLoader it gets even more complex to clean up the ClassValue so I'm testing another approach that simply disables the ClassValue in Groovy if the tests pass I will propose the ticket and the corresponding PR hoping that they will accept them otherwise I'm afraid that the only way is to keep the code as it is in camel-quarkus and apply your proposal in Quarkus Plarform. I don't think it is worth too much effort since once again it is a very specific use case and having Groovy classes loaded in the Quarkus Runtime ClassLoader may have an impact on the duration of the tests since it will load potentially several times the Groovy classes per module instead of only once.

@essobedo
Copy link
Contributor Author

essobedo commented Jul 3, 2023

The related ticket in Quarkus quarkusio/quarkus#34446

@ppalaga
Copy link
Contributor

ppalaga commented Jul 3, 2023

Thanks for investigating @essobedo!

@essobedo essobedo force-pushed the 4894/make-groovy-dsl-its-platform-compliant branch 3 times, most recently from 50ef389 to 75abb1c Compare July 7, 2023 12:07
@essobedo essobedo force-pushed the 4894/make-groovy-dsl-its-platform-compliant branch from 75abb1c to 6459e3a Compare July 7, 2023 12:14
@essobedo essobedo force-pushed the 4894/make-groovy-dsl-its-platform-compliant branch from 6459e3a to c8ebe7e Compare July 7, 2023 15:44
@essobedo essobedo marked this pull request as ready for review July 7, 2023 15:44
@jamesnetherton jamesnetherton merged commit b540eaf into quarkus-main Jul 18, 2023
@jamesnetherton jamesnetherton deleted the 4894/make-groovy-dsl-its-platform-compliant branch July 31, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants