Fixes #2626 : Introduce MockSettings.mockMaker

[JojOatXGME]

I implemented a prototype to fix #2626 as suggested in #2589 (comment). The basics doesn't seem that difficult, but I could have missed some edge cases. I mostly skipped documentation and tests for now. Furthermore, I ignored that ByteBuddyMockMaker and InlineByteBuddyMockMaker are not exposed by the API, so we still have to adjust the API to allow setting the mock maker without accessing these classes.

Personally, I don't need this functionality right now. However, if you think it would be a worthwhile addition, I can still try to help with that topic.

This pull request allows users of Mockito to choose different MockMakers per mock. This feature could ease the migration from one MockMaker to another in a big project.

SomeClass subclassMock = Mockito.mock(SomeClass.class, Mockito.withSettings()
    .mockMaker(AwesomeMockMaker.class));

PS: The Gradle task spotlessApply failed on my system with “java.lang.NoClassDefFoundError: Could not initialize class com.google.googlejavaformat.java.java14.Java14InputAstVisitor”. Probably because it couldn't find BindingPatternTree. Not sure if it is a local problem or something with the project configuration.

Checklist

• Read the contributing guide
• PR should be motivated, i.e. what does it fix, why, and if relevant how
• If possible / relevant include an example in the description, that could help all readers
  including project members to get a better picture of the change
• Avoid other runtime dependencies
• Meaningful commit history ; intention is important please rebase your commit history so that each
  commit is meaningful and help the people that will explore a change in 2 years
• The pull request follows coding style
• Mention Fixes #<issue number> in the description if relevant
• At least one commit should mention Fixes #<issue number> if relevant

[JojOatXGME]

I was able to run spotlessApply from the terminal (PowerShell on Windows). I previously tried to run the task from IntelliJ which for some reason was using a development build of JDK 14 (with Project Panama). 😅

[codecov-commenter]

Codecov Report

Merging #2701 (181d961) into main (0753d48) will increase coverage by 0.13%.
The diff coverage is 92.50%.

@@             Coverage Diff              @@
##               main    #2701      +/-   ##
============================================
+ Coverage     86.04%   86.18%   +0.13%     
- Complexity     2815     2828      +13     
============================================
  Files           320      320              
  Lines          8529     8583      +54     
  Branches       1053     1060       +7     
============================================
+ Hits           7339     7397      +58     
+ Misses          913      905       -8     
- Partials        277      281       +4     

Impacted Files	Coverage Δ
src/main/java/org/mockito/Mock.java	100.00% <ø> (ø)
src/main/java/org/mockito/Mockito.java	96.10% <ø> (ø)
...to/internal/configuration/SpyAnnotationEngine.java	98.41% <ø> (ø)
...guration/injection/SpyOnInjectedFieldsHandler.java	84.21% <ø> (ø)
src/main/java/org/mockito/plugins/MockMaker.java	87.50% <ø> (ø)
...ernal/configuration/plugins/PluginInitializer.java	66.66% <66.66%> (-0.78%)	⬇️
.../main/java/org/mockito/internal/util/MockUtil.java	91.66% <87.17%> (-6.49%)	⬇️
...rc/main/java/org/mockito/internal/MockitoCore.java	94.26% <100.00%> (+0.07%)	⬆️
...nternal/configuration/MockAnnotationProcessor.java	97.29% <100.00%> (+0.15%)	⬆️
...l/configuration/plugins/DefaultMockitoPlugins.java	91.66% <100.00%> (+1.34%)	⬆️
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[TimvdLippe]

Nice work! Definitely getting there, but left some initial review comments. Looking forward to this feature 😄

[JojOatXGME]

Here are my changes. I hope I am able to respond faster the next time. We also have to think about the API. Should the user be able to provide arbitrary mock makers (as right now in the PR), or should we make it more specific to the upcoming migration.

Generic

withSettings().mockMaker(SomeMockMaker.class)

In this case, we need to expose the internal mock makers somehow, or provide separate methods in the settings interface.

Specific

withSettings().useSubclassMockMaker()

Keeping the interface more specific to the upcoming migration would make it easier to deprecate and remove the feature in the future if it causes maintenance problems for some reason.

[TimvdLippe]

Nice, yes this is looking a lot better. Very happy about this design choice, this looks natural for loading custom mockmakers as well. I like your solution of MockMakers.java, since these strings were kinda magic at first and now we have a proper dynamic API for them as well.

Some last minor changes, but the overall design LGTM now.

We should also include a nice mention in our main Javadoc. Can you add section 53 to Mockito.java? Something along the lines of:

 * <h3 id="53">53. <a class="meaningful_link" href="#mock_annotation_mockmaker" name="mock_annotation_mockmaker">
 *  New mockmaker attribute for @Mock annotation and <code>MockSettings.mockMaker(String)</code> methods (Since 4.6.0)</a></h3>
 *
 * You can now customize the mockmaker for a single mock, either using `@Mock` annotation mockmaker attribute or
 * using `MockSettings.mockMaker(String)`. This can be useful for migrating to a different mockmakers
 * (which we are planning to do in Mockito 5 with https://github.com/mockito/mockito/issues/2589).
 *
 * <pre class="code"><code class="java">
 *   &#064;Mock(mockMaker = MockMakers.INLINE)
 *   Foo mock;
 *   // using MockSettings.withSettings()
 *   Foo mock = Mockito.mock(Foo.class, withSettings().mockMaker(MockMakers.SUBCLASS));
 * </code></pre>

Also, make sure you add a link to section 53 at the top of the Javadoc section.

[JojOatXGME]

I made some adjustments. I want to update the Javadoc later this week. There are also a few remaining TODOs I would like to discuss before merging this pull request.

[JojOatXGME]

@TimvdLippe, I have finished my work. If you have no further suggestions, I would replace my ${NEXT_VERSION} placeholder with 4.8.0 (or should it be 4.7.1?) and rebase/squash everything into one commit. Or what is the process?

FYI: I noticed that the following test is flaky, even on the main branch.

mockito/src/test/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubsConcurrentTest.java

Lines 17 to 24 in 530558a

	@Test
	public void
	given_mock_with_returns_deep_stubs__when_called_concurrently__then_does_not_throw_concurrent_modification_exception() {
	for (int i = 0; i < 1000; i++) {
	Service mock = mock(Service.class, Answers.RETURNS_DEEP_STUBS);
	IntStream.range(1, 100).parallel().forEach(index -> mock.doSomething());
	}
	}

It sometimes throws an ArrayIndexOutOfBoundsException in InvocationContainerImpl.getStubbingsAscending. The exception seems to be caused by a race condition. While the thread which throws the exception is reading InvocationContainerImpl.stubbed, another thread seems to add or remove items from the list.

[TimvdLippe]

@JojOatXGME Great work! The next version will be 4.8.0, so let's use that in the documentation 😄

Regarding the flaky test: if you have time to fix it, please go ahead. But for now, since we retry our tests, I think we can keep it as-is for now. Not ideal, I know...

[JojOatXGME]

@TimvdLippe I replaced my version placeholder and rebased and squashed my commits. I completely forgot your suggestion while writing the Javadoc, so if you have any final remarks or prefer another wording, I am happy to make adjustments. Otherwise, I think there is nothing left to be done for this PR.

[TimvdLippe]

Woohoo! Great feature 😄

Running the final checks on CI and then I will merge.

[TimvdLippe]

Nice test!

[TimvdLippe]

Released as part of 4.8.0 🎉