Codecov Report

Merging #1006 into release/2.x will increase coverage by <.01%.
The diff coverage is 100%.

@@                Coverage Diff                @@
##             release/2.x    #1006      +/-   ##
=================================================
+ Coverage          86.86%   86.86%   +<.01%     
- Complexity          2290     2291       +1     
=================================================
  Files                287      287              
  Lines               5801     5803       +2     
  Branches             684      684              
=================================================
+ Hits                5039     5041       +2     
  Misses               571      571              
  Partials             191      191

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7567f77...985e34a. Read the comment docs.

Hm, I am not sure about this change. Preferably I wouldn't want to make this change, as the PluginRegistry is supposed to run only once. Can't you make a delegate-MockMaker for your usecase, which dynamically determines which core MockMaker to delegate to?

hmm @TimvdLippe, That is essentially my work around for now in Powermock. What happens in the case where we have multiple MockMakers in the classpath? We wouldn't have control to make sure that the delegate is used since it would be whichever one ClassLoader.getResource() returns?

Is there something that depends on PluginRegistry not changing that I am missing? Or would you be more comfortable making our instance of mockmaker not final and only reloading that?

We have had several issues with loading and illogical exceptions related to the Registry. Therefore I would like to avoid introducing multiple loading stages, where again stuff can go wrong.

That's my 2 cents though, let's see what the others have to say about this.

I also think that dynamic reloading plugins it is overhead and needless feature. I don't see any case when it is required. All tests in one test class always should be run with one MockMaker. If speak about case of using with PowerMock, then PowerMock reloads classes for each test class, so it's easy to implement DelegateMockMaker to delegate to Mockito MockMaker when test is run without PowerMock and use PowerMockMockMaker is case if the test is started with using PowerMockRunner.

It will be better to have a link to default/parent Mockito MockMaker. It will help implement delegate-MockMaker with logic same as ClassLoader logic. I mean parent-first for parent-last.

Writing a DelegateMockMaker is simple but what where would one put it?

Would we essentially make PowerMockMaker the delagate in PowerMock? What happens when a user has a 3rd MockMaker in their classpath? Now the user needs to write a delegate to another as well as writing a switch to turn off all other MockMakers. IMO that is needless overhead for someone trying to turn on/off something for different test classes. Right now PowerMock is not on ByteBuddy so b/c it is on the classpath, someone cannot take advantage of Mockito2 for tests that do not require PowerMock. They would be forced to write their own MockMaker and switch. If we cannot reload plugins, the switch is essentially just hardcoding things on and off. We are now forcing our users to understand implementation details of dependencies they need and to write delegates and switches.

IMO that is much less idea that allowing one to choose to dynamically reload plugins when they choose. It would not change default behavior. If one reloads halfway through a test class, then we may get some weird messaging but we already get that when one expects Mockito2 features.

I am closing this PR for now. We are planning to address the MockMaker issues in Mockito 3, since we are now shipping multiple versions and there is no proper way to choose one for a test. For example, if just one test requires the InlineMockMaker. Stay tuned and thanks for your patience!