Migrate Jmockit VerificationsInOrder to Mockito

[shivanisky]

What's changed?

Add feature to migrate Jmockit VerificationsInOrder to mockito https://javadoc.io/doc/org.jmockit/jmockit/latest/mockit/VerificationsInOrder.html
https://www.javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/InOrder.html

What's your motivation?

• Incompatibility of JMockit, JaCoCo and Java 17 jmockit/jmockit1#729 (comment)

Anyone you would like to review specifically?

@sambsnyd @timtebeek @tinder-dthomson as FYI

Any additional context

Only JMockit Expectations Delegate left to migrate.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[timtebeek]

Great to see you follow through with the penultimate feature needed to complete the JMockit migrations. I've done a very quick round of comments from GitHub; will need to look in more detail later.

[timtebeek]

Suggested change

//private static final String VERIFY_IN_ORDER_TEMPLATE_PREFIX = "InOrder inOrder = inOrder(";

[shivanisky]

Ah yes - this is one issue I'd like to get input on
inOrder(mock) works, but
InOrder inOrder = inOrder(mock) causes error even though I have the right compilation options and the right import. Hence I am not able to get all the tests passing (have disabled for now).

Caused by: java.lang.IllegalArgumentException: Could not parse as Java
	at org.openrewrite.java.internal.template.JavaTemplateParser.lambda$compileTemplate$13(JavaTemplateParser.java:264)
	at java.base/java.util.Optional.orElseThrow(Optional.java:403)
	at org.openrewrite.java.internal.template.JavaTemplateParser.compileTemplate(JavaTemplateParser.java:264)
	at org.openrewrite.java.internal.template.JavaTemplateParser.lambda$parseBlockStatements$9(JavaTemplateParser.java:176)
	at org.openrewrite.java.internal.template.JavaTemplateParser.lambda$cacheIfContextFree$14(JavaTemplateParser.java:287)
	at org.openrewrite.java.internal.template.JavaTemplateParser.cache(JavaTemplateParser.java:308)
	at org.openrewrite.java.internal.template.JavaTemplateParser.cacheIfContextFree(JavaTemplateParser.java:287)
	at org.openrewrite.java.internal.template.JavaTemplateParser.parseBlockStatements(JavaTemplateParser.java:171)
	at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.lambda$visitBlock$1(JavaTemplateJavaExtension.java:112)
	at org.openrewrite.internal.ListUtils.lambda$flatMap$0(ListUtils.java:261)
	at org.openrewrite.internal.ListUtils.flatMap(ListUtils.java:205)
	at org.openrewrite.internal.ListUtils.flatMap(ListUtils.java:261)
	at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitBlock(JavaTemplateJavaExtension.java:110)
	at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitBlock(JavaTemplateJavaExtension.java:55)
	at org.openrewrite.java.tree.J$Block.acceptJava(J.java:838)
	at org.openrewrite.java.tree.J.accept(J.java:59)
	at org.openrewrite.TreeVisitor.visit(TreeVisitor.java:250)
	... 58 more

[shivanisky]

Ah all good, that parse error went away - thanks, maybe all that was needed was to sync with main! Now just only one test failing with some strange formatting issue, just an empy new line between statements in test whenThreeBlocks g. https://ge.openrewrite.org/s/pyyay4x3zaeui/tests/task/:test/details/org.openrewrite.java.testing.jmockit.JMockitVerificationsInOrderToMockitoTest/whenThreeBlocks()?top-execution=1

Not due to having 3 blocks, but due to having a very short InOrder block of 2 lines.

[timtebeek]

Looks like we're very close, with only one failure left on whitespace

JMockitVerificationsInOrderToMockitoTest > whenThreeBlocks() FAILED
    org.opentest4j.AssertionFailedError: [Unexpected result in "MyTest.java":

diff --git a/MyTest.java b/MyTest.java
index 99fe3e8..bd0a50f 100644
--- a/MyTest.java
+++ b/MyTest.java
@@ -18,11 +18,15 @@ 
         inOrder.verify(obj).wait();

         obj.wait();
+
         InOrder inOrder1 = inOrder(obj);
+
         inOrder1.verify(obj).wait();

         obj.wait(10L, 10);
+
         InOrder inOrder2 = inOrder(obj);
+
         inOrder2.verify(obj).wait(anyLong(), anyInt());
     }
 }
\ No newline at end of file

expected:

import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import static org.mockito.Mockito.*;

@ExtendWith(MockitoExtension.class)
class MyTest {
    @Mock
    Object obj;

    void test() {
        obj.wait(10L, 10);
        obj.wait();
        InOrder inOrder = inOrder(obj);
        inOrder.verify(obj).wait(anyLong(), anyInt());
        inOrder.verify(obj).wait();

        obj.wait();
        InOrder inOrder1 = inOrder(obj);
        inOrder1.verify(obj).wait();

        obj.wait(10L, 10);
        InOrder inOrder2 = inOrder(obj);
        inOrder2.verify(obj).wait(anyLong(), anyInt());
    }
}

but was:

import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import static org.mockito.Mockito.*;

@ExtendWith(MockitoExtension.class)
class MyTest {
    @Mock
    Object obj;

    void test() {
        obj.wait(10L, 10);
        obj.wait();
        InOrder inOrder = inOrder(obj);
        inOrder.verify(obj).wait(anyLong(), anyInt());
        inOrder.verify(obj).wait();

        obj.wait();

        InOrder inOrder1 = inOrder(obj);

        inOrder1.verify(obj).wait();

        obj.wait(10L, 10);

        InOrder inOrder2 = inOrder(obj);

        inOrder2.verify(obj).wait(anyLong(), anyInt());
    }
}

[shivanisky]

Yes … but possibly this white space issue is unrelated to this PR. As I think I’ve seen it in Expectations migration, maybe. Will have to check it out. It’s happening in a specific case only.

…

On Wed, 11 Dec 2024 at 10:28 AM, Tim te Beek ***@***.***> wrote: Looks like we're very close, with only one failure left on whitespace JMockitVerificationsInOrderToMockitoTest > whenThreeBlocks() FAILED org.opentest4j.AssertionFailedError: [Unexpected result in "MyTest.java": diff --git a/MyTest.java b/MyTest.java index 99fe3e8..bd0a50f 100644--- a/MyTest.java+++ b/MyTest.java@@ -18,11 +18,15 @@ inOrder.verify(obj).wait(); obj.wait();+ InOrder inOrder1 = inOrder(obj);+ inOrder1.verify(obj).wait(); obj.wait(10L, 10);+ InOrder inOrder2 = inOrder(obj);+ inOrder2.verify(obj).wait(anyLong(), anyInt()); } } \ No newline at end of file expected: import org.junit.jupiter.api.extension.ExtendWith;import org.mockito.InOrder;import org.mockito.Mock;import org.mockito.junit.jupiter.MockitoExtension; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class)class MyTest { @mock Object obj; void test() { obj.wait(10L, 10); obj.wait(); InOrder inOrder = inOrder(obj); inOrder.verify(obj).wait(anyLong(), anyInt()); inOrder.verify(obj).wait(); obj.wait(); InOrder inOrder1 = inOrder(obj); inOrder1.verify(obj).wait(); obj.wait(10L, 10); InOrder inOrder2 = inOrder(obj); inOrder2.verify(obj).wait(anyLong(), anyInt()); } } but was: import org.junit.jupiter.api.extension.ExtendWith;import org.mockito.InOrder;import org.mockito.Mock;import org.mockito.junit.jupiter.MockitoExtension; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class)class MyTest { @mock Object obj; void test() { obj.wait(10L, 10); obj.wait(); InOrder inOrder = inOrder(obj); inOrder.verify(obj).wait(anyLong(), anyInt()); inOrder.verify(obj).wait(); obj.wait(); InOrder inOrder1 = inOrder(obj); inOrder1.verify(obj).wait(); obj.wait(10L, 10); InOrder inOrder2 = inOrder(obj); inOrder2.verify(obj).wait(anyLong(), anyInt()); } } — Reply to this email directly, view it on GitHub <#632 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BI27SFS5KOSDXQLLQDTROMD2E52KLAVCNFSM6AAAAABRGOMPE6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZTGIYTCNRQGY> . You are receiving this because you were assigned.Message ID: ***@***.***>

[timtebeek]

Ah check! Thanks for looking into it; also let me know if you'd like me to take over. The way I see it there's a few options:

1. Counteract the left over whitespace in the recipe
2. Try to resolve the issue upstream somewhere
3. Adjust the tests to match the current output

All three could be a viable way to merge this PR, and don't necessarily rule out one another; I think folks would prefer to see these case migrated, even if there are excess newlines, for now. Then if the issue is fixed upstream we can revisit. Let me know what you find and which direction you want to go. :)

[shivanisky]

Hi Tim I’m more than happy for you to take over. A bit tied up with work (java migration hehe). Any of those options look good. It seems the issue is related to the number of lines before migration vs the number of lines in the new code. I couldn’t see any issue in the recipe but I’m still new to this. Suspect issue is in the framework. Thanks Shivani

…

On Wed, 11 Dec 2024 at 10:38 PM, Tim te Beek ***@***.***> wrote: Ah check! Thanks for looking into it; also let me know if you'd like me to take over. The way I see it there's a few options: 1. Counteract the left over whitespace in the recipe 2. Try to resolve the issue upstream somewhere 3. Adjust the tests to match the current output All three could be a viable way to merge this PR, and don't necessarily rule out one another; I think folks would prefer to see these case migrated, even if there are excess newlines, for now. Then if the issue is fixed upstream we can revisit. Let me know what you find and which direction you want to go. :) — Reply to this email directly, view it on GitHub <#632 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BI27SFUAW6CJ7QCMSIDZWMT2FAP4DAVCNFSM6AAAAABRGOMPE6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZVGY3TENJZGQ> . You are receiving this because you were assigned.Message ID: ***@***.***>

[timtebeek]

Thanks a lot @shivanisky ! Great to see this verifications in order covered as well.

I explored the whitespace issue briefly, and noticed a missing semicolon, but that failed to resolve the issue. We'll accept that for now and can circle back later if needed. I don't think folks will stumble over this in the migration, so then it's better to see it covered.