Add migration of JMockit NonStrictExpectations blocks as well as JMockit junit 4 runner

[shivanisky]

Add migration of JMockit NonStrictExpectations blocks as well as JMockit junit 4 runner. Info on NonStrictExpectations: https://javadoc.io/doc/org.jmockit/jmockit/1.22/mockit/NonStrictExpectations.html

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

@tinder-dthomson @timtebeek

Have you considered any alternatives or workarounds?

Any additional context

Checklist

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

[timtebeek]

This looks good to me overall! I will follow-up with another PR to completely remove @Nullable, I'd rather not block you any longer.

We can keep the internal nullable annotations; I've restored a few in these classes already with https://github.com/openrewrite/rewrite-testing-frameworks/pull/546/commits/e906d82c4643e69e47039c7b1ea114ec3396cd79. The idea is that we add non null by default to package-info.java, and then only have to be explicit using our internal nullable annotation as needed.

Thanks both! Great to see this come along.

[shivanisky]

This is a very good point - perhaps times is honoured as an Expectation - it seems from the documentation this is what would happen.

https://javadoc.io/doc/org.jmockit/jmockit/1.22/mockit/NonStrictExpectations.html

“ A non-negative value assigned to this field will be taken as the exact number of times that invocations matching the current expectation should occur during replay.”

I haven’t seen a times in a NonStrictExpectation but should support it … it would get wiped for now. I’ll look at it next week.

I guess the clause to ignore times should be removed, and then if has times, don’t add the lenient() and also add corresponding verify with the times, as is already being done with Expectations.

I’ll try to add this next week.

On Sat, 6 Jul 2024 at 3:20 AM, tinder-dthomson @.***> wrote:

@.**** commented on this pull request.

In src/main/java/org/openrewrite/java/testing/jmockit/JMockitBlockRewriter.java https://github.com/openrewrite/rewrite-testing-frameworks/pull/546#discussion_r1667010699 :

@@ -131,6 +132,11 @@ private void rewriteMethodInvocation(List statementsToRewrite) { rewriteResult(invocation, mockInvocationResults.getResults()); }

• if (blockType == NonStrictExpectations) {

It's a bit unclear what the behavior is, due to this statement:

"The difference from regular expectations https://javadoc.io/static/org.jmockit/jmockit/1.22/mockit/Expectations.html is that a non-strict recorded expectation is allowed to have no matching invocations (unless otherwise specified)."

I wonder if this means that if times is specified, does it get honored? I'm fine with making this assumption for now but I wouldn't be surprised if JMockit has the ability to mix both.

— Reply to this email directly, view it on GitHub https://github.com/openrewrite/rewrite-testing-frameworks/pull/546#discussion_r1667010699, or unsubscribe https://github.com/notifications/unsubscribe-auth/BI27SFSO4XMMCR4PVKL44RLZK3IXJAVCNFSM6AAAAABKK6X6QOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRRGA2TKOJWG4 . You are receiving this because you were assigned.Message ID: @.*** com>

[tinder-dthomson]

This looks good to me overall! I will follow-up with another PR to completely remove @Nullable, I'd rather not block you any longer.

We can keep the internal nullable annotations; I've restored a few in these classes already with e906d82. The idea is that we add non null by default to package-info.java, and then only have to be explicit using our internal nullable annotation as needed.

Thanks both! Great to see this come along.

Thanks for chiming in! I was unaware of your previous touches related to @Nullable. It's unfortunate the package contains internal if we're open to it being used by recipe authors, but this is a better alternative to using javax or jakarta package roots imo.