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
Allow use of Mockito jar as an agent. #3137
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3137 +/- ##
============================================
- Coverage 85.48% 85.44% -0.05%
- Complexity 2892 2910 +18
============================================
Files 329 334 +5
Lines 8826 8869 +43
Branches 1095 1099 +4
============================================
+ Hits 7545 7578 +33
- Misses 994 1000 +6
- Partials 287 291 +4
☔ View full report in Codecov by Sentry. |
…ble instrumentation, considering upcoming restrictions on dynamic attach.
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.
Changes look good to me. Should we also update our CI to run on 21 or add a regression test in a different way to ensure we remain compatible?
Likely a good idea. But probably in a separate PR? |
Sure. That works for me. As long as we make sure it does fix the problem on Java 21. |
|
||
public class MockitoAgent { | ||
|
||
private static Instrumentation instrumentation; |
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.
I would say this should be volatile
or guarded by a lock.
You can't guarantee visibility of the static field, because you don't know which thread will call premain
and which one calls getInstrumentation()
.
This is open for two months now. Any ETA when this will hit an actual mockito release? |
Revisit the mockito setup (this can be improved once mockito/mockito#3137 has landed and was released with mockito). For now, we add the bytebuddy agent directly. Also update spotbugs and junit, which is needed to pass build under Java 22-ea.
We are awaiting the resolution of the Maven team on what the official migration path for projects such as Mockito will be: #3037 (comment) |
Revisit the mockito setup (this can be improved once mockito/mockito#3137 has landed and was released with mockito). For now, we add the bytebuddy agent directly. Also update spotbugs and junit, which is needed to pass build under Java 22-ea.
Adds premain method such that Mockito can be used as an agent and enable instrumentation, considering upcoming restrictions on dynamic attach.