-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Execute test plan with the deployment classloader as the TCCL #24440
Execute test plan with the deployment classloader as the TCCL #24440
Conversation
I should add a test though |
a0d8601
to
d052ee2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, test would be nice.
Apart from that: LGTM; I tested it successfully in my real project and Stuart already said it's the proper way.
I'm only asking myself what's left for which we need the augmentation classloader (set in ModuleTestRunner
)?
PS: IMO it makes sense to backport, but I'll leave this to you. |
@aloubyansky do you plan on adding the test soon or should we merge? |
Yes, I'll look into adapting the reproducer provided by @famod |
@aloubyansky I will merge this tonight latest as I want this in CR1. If you don't have the time to write the test now, no biggie, we can include it later. |
Unfortunately, I couldn't create a test case that would fail. I tried using |
Fix #24229