openrewrite / rewrite-testing-frameworks

OpenRewrite recipes that perform common Java testing migration tasks.
Apache License 2.0
77 stars 72 forks source link

JMockit to Mockito #325

Closed nmck257 closed 1 week ago

nmck257 commented 1 year ago

JMockit remains popular, but has not had a release since 2019, and has various compatibility issues with Java 11+.

As Java 8 loses LTS status, developers would likely benefit from some recipe(s) to facilitate migration from JMockit to Mockito

klauerm commented 1 year ago

From my first research I found the following, that the recipe shall do:

  1. Replace JMockit imports with Mockito imports: Replace JMockit imports in your test classes with Mockito imports. For example, replace:

    import mockit.*;

    with

    import org.mockito.Mockito;
    import org.mockito.junit.MockitoJUnitRunner;
  2. Refactor test class annotations: Replace JMockit's @RunWith(JMockit.class) with Mockito's @RunWith(MockitoJUnitRunner.class).

  3. Refactor mocks: Replace @Mocked with @Mock for mock objects. For example, replace:

    @Mocked
    private MyClass myClassMock;

    with

    @Mock
    private MyClass myClassMock;
  4. Refactor test method setup: In JMockit, you would typically use new Expectations(), new StrictExpectations(), or new Verifications() blocks to set up and verify mock behaviors. In Mockito, you'll use methods like when(), thenReturn(), verify(), and times().

For example, if you had a JMockit expectation like this:

new Expectations() {{
    myClassMock.myMethod(input);
    result = output;
}};

You would refactor it to use Mockito like this:

Mockito.when(myClassMock.myMethod(input)).thenReturn(output);
  1. Refactor verifications: If you had a JMockit verification like this:
    new Verifications() {{
    myClassMock.myMethod(input);
    times = 1;
    }};

    You would refactor it to use Mockito like this:

    Mockito.verify(myClassMock, Mockito.times(1)).myMethod(input);

@nmck257 Is there anything else, that is missing from your point of view?

nmck257 commented 1 year ago

Honestly, I've never used JMockit myself, but that looks like an excellent scope to start with. :)

Note that RunWith is JUnit4 syntax, so we may want to be mindful about how this recipe should interact with the JUnit 5 migration recipe(s)

hazendaz commented 1 year ago

This would be great! I'm currently maintaining jmockit and have been for few years now. Only to keep it alive and working on newer jdks. The original author blocked me, went MIA, was not very OSS friendly at all. I have most if not all PRs people have raised into my fork, its identical otherwise. But having ability to rewrite it back to mockito would be best option. In my company we were forced to move to jmockit in 2015 with claims mockito didn't do this or that which were 100% wrong. But again we were forced. Now with it dead and I have it on life support, its only going to work not get new features and the code base is really java 7 based mostly. Its javaEE based too but I'll get it to jakartaEE. Usage wise though from a rewrite, it should not change how it looks and having gone from mockito all repo with 90% coverage in 2015 entirely to jmockit, going backwards should not be that hard. There are certainly items to think more about such as Deencapsulation as many stayed on old versions where that was removed. The natural replacement at the time was Powermock.Whitebox. But I don't know what that should be today as powermock is also dead.

I can try to help out but don't have a lot of time to do so. But wanted to chime in as this is a good suggestion.

hazendaz commented 1 year ago

The supported copy of the repo is here https://github.com/hazendaz/jmockit1. It is the only one that properly works with jacoco and jdk11+. It should work through jdk 20 currently for reference. The master there is broken on GHA for the moment but will fix in coming week or so.

hazendaz commented 1 year ago

Sorry and finally you can find the releases to show its been done by me since 2020 -> https://search.maven.org/artifact/com.github.hazendaz.jmockit/jmockit

timtebeek commented 9 months ago

Figured update this issue that we've since had a steady stream of work in migration recipes from JMockit to Mockito, mostly from @tinder-dthomson 🙏🏻

There's already a first recipe available for folks to use:

Curious to know what else we would need to cover before we can close this issue. Typically it's hard to say at what point we're done with an issue like this. We can also opt to close this issue and open up new issues for specific extensions if that makes more sense.

tinder-dthomson commented 9 months ago

@timtebeek Hey there! My recommendation is we use this issue as a tracker for the remaining items to resolve.

I have one set of changes (locally) that improves handling of parameterized class arguments like Class<T>, such that the type information is not lost. I'm sure there are more cases like this that I haven't yet resolved, but unfortunately you don't know what you don't know.

I'm am aware, however, of these big ticket items:

- Verifications blocks (including argument capturing) - Static Method Mocks (in an Expectations or Verifications block) - Spies (as parameters to Expectations blocks) - MockUp blocks

There are also many JMockit features that we haven't even mentioned.

kernanj commented 8 months ago

@timtebeek Hey there! My recommendation is we use this issue as a tracker for the remaining items to resolve.

I have one set of changes (locally) that improves handling of parameterized class arguments like Class<T>, such that the type information is not lost. I'm sure there are more cases like this that I haven't yet resolved, but unfortunately you don't know what you don't know.

I'm am aware, however, of these big ticket items:

- Verifications blocks (including argument capturing) - Static Method Mocks (in an Expectations or Verifications block) - Spies (as parameters to Expectations blocks) - MockUp blocks

There are also many JMockit features that we haven't even mentioned.

Hi there,

This is a good list to work from. The static method mock is a big one and I've seen a few different ways that this is set up using JMockit.

1. Class passed to expectation

    new Expectations(SomeClassToBeMocked.class) {
        SomeClassToBeMocked.staticMethod();
        result="SOMETHING";
    };

Passing a Class in the Expectations constructor has been removed since JMockit 1.48

2. Mocked method parameter/field

@Test
void testWithStaticMock(@Mocked SomeClassToBeMocked someClassToBeMocked) {
    new Expectations() {
        SomeClassToBeMocked.staticMethod();
        result="SOMETHING";
    };
}

What would a Mockito equivalent look like?

The Mockito equivalent might look something like this

    try (MockedStatic mocked = mockStatic(SomeClassToBeMocked.class)) {
        mocked.when(SomeClassToBeMocked::staticMethod).thenReturn("SOMETHING");
        // Code executing static method
        ...
    }

Mockito docs suggest keeping the mock in the scope of a try-with-resources block but how would we determine what should be in scope?

If the mock is defined as a method parameter like above, do we avoid the try-with-resources, create the MockedStatic at the start of the test and close at the end of the test?

If a @Mocked field is defined, do we create the MockedStatic in the @BeforeEach and close in the @AfterEach?

tinder-dthomson commented 8 months ago

@kernanj I've thought about this quite a bit. The best approach that works with the existing recipe I've come up with looks like this:

Using your example above, the recipe processing would go like this:

@Test
void testWithStaticMock(@Mocked SomeClassToBeMocked someClassToBeMocked) {
    new Expectations() {
        SomeClassToBeMocked.staticMethod();
        result="SOMETHING";
        someMockedVariable.getAnotherThing();
        result="ANOTHER_THING";
    };
}

// after rewriting static methods, before rewriting `Expectations` blocks

@Test
void testWithStaticMock(@Mocked SomeClassToBeMocked someClassToBeMocked) {
    try (MockedStatic<SomeClassToBeMocked> mockedStatic = mockStatic(SomeClassToBeMocked.class)) {
        mockedStatic.when(() -> SomeClassToBeMocked.staticMethod()).thenReturn("ANOTHER_THING");
        new Expectations() {
            someMockedVariable.getAnotherThing();
            result="ANOTHER_THING";
        }; 
    }
}

// after rewriting `Expectations` blocks

@Test
void testWithStaticMock(@Mocked SomeClassToBeMocked someClassToBeMocked) {
    try (MockedStatic<SomeClassToBeMocked> mockedStatic = mockStatic(SomeClassToBeMocked.class)) {
        mockedStatic.when(() -> SomeClassToBeMocked.staticMethod()).thenReturn("SOMETHING");
        when(someMockedVariable.getAnotherThing()).thenReturn("ANOTHER_THING");
    }
}

This would re-use the heavy lifting of the ExpectationsBlockRewriter while fitting static method mocking in, likely as a separate "Rewriter" class.

Finally, regarding wrapping the entire test method using @BeforeEach and @AfterEach... I would avoid that. There's no guarantee that the static class isn't used before the first expectation is recorded. Better to simply put the rest of the test method body within it.

kernanj commented 8 months ago

@tinder-dthomson We would need to put the remainder of the test in scope of the try with resources though. I've given an example below where there are multiple static methods being mocked in an expectation block.

public class SomeClassToBeMocked {
    public static String someStaticMethod() {
        return "I'm a real static method";
    }
}
public class SomeOtherClassToBeMocked {
    public static String otherStaticMethod() {
        return "I'm another real static method";
    }
}
    @Test
    void testStaticMethod() {

        try (MockedStatic<SomeClassToBeMocked> someStaticToBeMocked = mockStatic(SomeClassToBeMocked.class);
             MockedStatic<SomeOtherClassToBeMocked> someOtherClassToBeMocked = mockStatic(SomeOtherClassToBeMocked.class)) {

            someStaticToBeMocked.when(() -> SomeClassToBeMocked.someStaticMethod()).thenReturn("I'm a mocked static mocked");
            someOtherClassToBeMocked.when(() -> SomeOtherClassToBeMocked.otherStaticMethod()).thenReturn("I'm another static method that has been mocked");

            // Mocked return is used as we need the caller to be in scope of try-with-resources
            System.out.println(SomeClassToBeMocked.someStaticMethod());
        }

        // Not in scope so real static is called
        System.out.println(SomeOtherClassToBeMocked.otherStaticMethod());
    }
kernanj commented 8 months ago

"All the remaining statements of the test method body, including the current Expectations statement, should be written as the try-with-resources block body" missed this point you made above, I think we're on the same page here 🙂

mrniko commented 7 months ago

You can use this Jmockit fork https://github.com/hazendaz/jmockit1/ It's maintained and supports Java 11+

SiBorea commented 2 months ago

hi @tinder-dthomson I have created a JMockitMockUpToMockito recipe and want to contribute, what should I do? Just create a PR?

timtebeek commented 1 week ago

With https://github.com/openrewrite/rewrite-testing-frameworks/pull/599 merged just now I think we've covered a good deal op JMockit features already. I propose we close this overarching issue and open new separate issues for additional features folks might like to cover. Thanks all for the work done here! It's been great seeing this come together with so much community contributions.