Mock resolver plugin #2042
Mock resolver plugin #2042
Conversation
| for (MockResolver mockResolver : Plugins.getMockResolvers()) { | ||
| mock = mockResolver.resolve(mock); | ||
| } | ||
|
|
||
| MockHandler handler = mockMaker.getHandler(mock); |
There was a problem hiding this comment.
Nit: extract this out in a private method and use that below as well.
There was a problem hiding this comment.
Fully agree, I hope for some feedback first, too.
|
|
||
| @MockitoSettings(strictness = Strictness.WARN) | ||
| @ExtendWith(MockitoExtension.class) | ||
| class MockResolverTest { |
There was a problem hiding this comment.
Can we add a test that uses multiple resolvers? I can think of the following scenarios:
- Multiple resolvers, no match
- Multiple resolvers, one match
- Multiple resolvers, multiple match
For scenario 3, should we throw maybe? What if it is order-dependent? Should we continue iterating over the resolvers until none match?
There was a problem hiding this comment.
I think multiple resolvers should be allowed. You can technically add several in your specified order and decompose multiple levels of nesting. It's of course an obscure scenario, but it makes sense to not forbid it since it would be extra work. I can add the tests you suggest.
There was a problem hiding this comment.
Hmm, registering multiple converters in a test is not quite that easy since it would require multiple modules. I don't think its worth the noise to add this functionality just for testing such a simple corner case.
There was a problem hiding this comment.
Understood. How would that work when a user wants to integrate with multiple frameworks that all expose a resolver?
There was a problem hiding this comment.
They would all expose their resolver and they would be invoked in their registration order. For instance, let's say Hibernate offered a Mockito-based mocking API but returns Hibernate proxies on mocks. Those proxies are by definition never Spring Beans. But if one passed a Spring bean or a Hibernate proxy, both would traverse the resolver chain and be unpacked accordingly.
I assume that normally, only one resolver would apply to any wrapped mock but since Mockito should remain agnostic to this, that would be the best approach in my eyes.
Codecov Report
@@ Coverage Diff @@
## release/3.x #2042 +/- ##
=================================================
+ Coverage 84.89% 84.92% +0.03%
- Complexity 2704 2724 +20
=================================================
Files 325 325
Lines 8204 8320 +116
Branches 979 1009 +30
=================================================
+ Hits 6965 7066 +101
- Misses 968 983 +15
Partials 271 271 Continue to review full report at Codecov.
|
…third-party frameworks.
raphw
force-pushed
the
mock-resolver-plugin
branch
from
f585602
to
48d817f
Compare
raphw
force-pushed
the
mock-resolver-plugin
branch
from
683a60e
to
c81b028
Compare
| @@ -55,21 +56,25 @@ public static TypeMockability typeMockabilityOf(Class<?> type) { | |||
| return mock; | |||
| } | |||
|
|
|||
| public static <T> void resetMock(T mock) { | |||
| public static void resetMock(Object mock) { | |||
There was a problem hiding this comment.
Why were the generic parameters removed from this and the other methods? I understand that they are not used, but I wonder if there are unintended side-effects of this change.
There was a problem hiding this comment.
Just as a cleanup on the side since I am touching the code already. The generic type is erased to Object and has no other function, that's why I removed it. No impact on source or binary compatibility.
| * provided to the {@link org.mockito.Mockito}-DSL. This mechanism can be used by | ||
| * frameworks that provide mocks that are implemented by Mockito but which are wrapped | ||
| * by other instances to enhance the proxy further. | ||
| */ |
There was a problem hiding this comment.
This needs to be marked as @Incubating
There was a problem hiding this comment.
Arghs, I missed this and just hit merge. Sorry, we can still add this, though.
There was a problem hiding this comment.
Yes let's still add it.
Adds a plugin to allow for adding one or multiple mock resolvers. This way, instances that are provided to Mockito's DSL do no longer require to be the actual mocks but can also be proxies of mocks what is something already being applied by frameworks such as Spring.
This works today with the subclass mock maker by accident as we read the mock state from the mock instance via a method which gets proxied. If we ever find a more private appraoch this would however break Spring's Mockito use and it is already broken with the inline mock maker.
By this SPI, Spring could add an unproxy resolver to its Mockito build-up and make it's proxied mocks compatible with Mockito. Other frameworks could use the same approach if desired as this SPI is generic.
Closes #1980