Skip to content
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

The future of inline mock maker #1728

Closed
arturdryomov opened this issue Jun 11, 2019 · 6 comments
Closed

The future of inline mock maker #1728

arturdryomov opened this issue Jun 11, 2019 · 6 comments

Comments

@arturdryomov
Copy link

I‘ve been looking through MockMaker subclasses and noticed an interesting comment in org.mockito.internal.creation.bytebuddy.ByteBuddyMockMaker.

/**
 * ByteBuddy MockMaker.
 *
 * This class will serve as the programmatic entry point to all mockito internal MockMakers.
 * Currently the default and only mock maker is the subclassing engine, but with enough feedback we can later
 * promote the inlining engine for features like final class/methods mocks.
 *
 * The programmatic API could look like {@code mock(Final.class, withSettings().finalClasses())}.
 */

I wonder — does this still hold true? I’m asking mostly from experience described in #1532 and #1533. This lead to #1614 and an awkward API:

@After fun clearMocks() {
    Mockito.framework().clearInlineMocks()
}

If there are plans to make it default anyway it would be great to still make it opt-in or easily opt-out.

@TimvdLippe
Copy link
Contributor

I think we are interested in making it the default, but we are uncomfortable doing that right now. We will probably consider switching the default in Mockito 4, but that won't happen in the near future.

@TimvdLippe
Copy link
Contributor

@raphw could you post your thoughts on the status of the subclass and inline mockmakers, given the recent publication of https://openjdk.java.net/jeps/396 ?

@raphw
Copy link
Member

raphw commented Nov 2, 2020

So, the major change that is going to affect us will be the passing of Objenesis. Oracle makes a clear point about this possibility to disappear with serialization being the most common security threat. It will no longer be possible to instantiate any object without calling a constructor.

The inline mock maker avoids this problem by calling the constructor whilst instrumenting them not to run any code within them. Therefore, the inline mock maker can continue to keep up the current promise made by Mockito.

If we want to support subclass mock making, we will have to invoke any mock instances constructor in the future what we can do reflectively, but if a constructor just dereferences one value, we will end up throwing an exception when creating the mock. For big code bases, this is hardly feasible to migrate into. The only alternative will be to disable the strict seal what might also become impossible at some point.

The second issue is certainly the module system when used where the subclass mock maker will always require an explicit opening whereas the inline mock maker can and will continue to work out of the box.

These two issues will seperate the two mock makers from each other in the future.

@TimvdLippe
Copy link
Contributor

Thanks Rafael for the explanation. Based on that analysis, I guess our question to answer is: Do we want to maintain two mock makers instead of one?

Given the technical details, I would be inclined to choose the inline mock maker over the subclass mock maker, given that it is future-proof. We already hint towards that in our JavaDoc (https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#0.2 and the detailed section https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#39).

To aid our users, I think we need to make this call well in advance, to give users time to migrate to mockito-inline. We can then flip the switch in Mockito 4, but I would like to give users ample opportunity to discover any potential issues.

@mockito/developers could you let us know what you think about this?

@raphw
Copy link
Member

raphw commented Nov 2, 2020

Well, the inline mock maker is using the subclass mock maker to a large extend. You cannot, for example, mock an abstract class, without creating a subclass of it for the purpose of mocking. Therefore, the maintenance will not be reduced much by removing the official API to the subclass mock maker. As the subclass mock maker can technically be reinstated by tweaking the JVM (to an increasing degree), I think we should let it continue to live on.

If you have a limited amount of mocks and are willing to make the trade to use the subclass mock maker, this can also be a reasonable choice since the agent attachment takes a bit of time, unfortunately, especially when running single tests since the agent attachment has to be conducted for each JVM startup.

The more we approach Java 17, the more the inline mock maker makes sense, however. Maybe we should make the default JVM version-dependent at some point? Or is that too much magic?

@TimvdLippe
Copy link
Contributor

Ah, that is very interesting. I hadn't realized that particular relationship between the mock makers. My ideal solution would be something that is the easiest for users. E.g. it should "just" work. Ideally we have just 1 mock maker for that, but if that isn't possible, then we should at least handle it on the Mockito side.

That said, there seem to be a lot of subtleties involved and I am not sure if it is even feasible to reach such a situation. What I do want to prevent is users being confused and conflicting advice being scattered around the web. Therefore, if we have to maintain two mock makers, the guidance should be crystal clear in which situation which mock maker should be used and why.

Let's ponder on this a bit...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants