openrewrite / rewrite-testing-frameworks

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

org.junit.rules.ExpectedException not migrated as part of SpringBoot2JUnit4to5Migration #581

Open FieteO opened 2 months ago

FieteO commented 2 months ago

What version of OpenRewrite are you using?

I am using version 5.17.0 of the Migrate Spring Boot 2.x projects to JUnit 5 from JUnit 4 recipe

How are you running OpenRewrite?

package some.package;

import static org.junit.Assert.assertTrue;

import java.util.Arrays;
import java.util.Collection;

import org.junit.*;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

@RunWith(value = Parameterized.class)
public class SomeTest {

    public enum QueryStyle {
        FOO, BAR
    }

    @Rule
    public ExpectedException thrown = ExpectedException.none();

    @Parameterized.Parameter
    public QueryStyle queryStyle;

    @Parameterized.Parameters(name = "QueryStyle.{0}")
    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][] {{QueryStyle.FOO}, {QueryStyle.BAR}});
    }

    @Test
    public void test() throws Exception {
        assertTrue((queryStyle.equals(QueryStyle.FOO) || queryStyle.equals(QueryStyle.BAR)));
        throw new RuntimeException("exception");
        thrown.expectMessage(containsString("exception"));
    }
} 

What did you expect to see?

package some.package;

import org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.Assertions.assertThrows;

import java.util.Arrays;
import java.util.Collection;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.rules.ExpectedException;

public class SomeTest {

    public enum QueryStyle {
        FOO, BAR
    }

    public QueryStyle queryStyle;

    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][] {{QueryStyle.FOO}, {QueryStyle.BAR}});
    }

    @MethodSource("data")
    @ParameterizedTest(name = "QueryStyle.{0}")
    public void test(QueryStyle queryStyle) throws Exception {
        assertTrue((queryStyle.equals(QueryStyle.FOO) || queryStyle.equals(QueryStyle.BAR)));
        RuntimeException exception =
            assertThrows(RuntimeException.class, () -> {
                throw new RuntimeException("exception");
            });
        assertEquals("exception", exception.getMessage());
    }

    public void initSomeTest(QueryStyle queryStyle) {
        this.queryStyle = queryStyle;
    }
}

What did you see instead?

The declaration of @Rule public ExpectedException thrown is correctly removed, but the reference to thrown is not correctly replaced in the test method. Also the import for ExpectedException is not removed.

package some.package;

import org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.rules.ExpectedException;

import java.util.Arrays;
import java.util.Collection;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.rules.ExpectedException;

public class SomeTest {

    public enum QueryStyle {
        FOO, BAR
    }

    public QueryStyle queryStyle;

    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][] {{QueryStyle.FOO}, {QueryStyle.BAR}});
    }

    @MethodSource("data")
    @ParameterizedTest(name = "QueryStyle.{0}")
    public void test(QueryStyle queryStyle) throws Exception {
        assertTrue((queryStyle.equals(QueryStyle.FOO) || queryStyle.equals(QueryStyle.BAR)));
        throw new RuntimeException("exception");
        thrown.expectMessage(containsString("exception"));
    }

    public void initSomeTest(QueryStyle queryStyle) {
        this.queryStyle = queryStyle;
    }
}

I hope I have not forgotten anything in the code blocks. Also it may not be necessary to have this as a parameterized test. If the issue cannot be reproduced based on the provided snippets, than running the recipe on the provided repository should reproduce the issue.

timtebeek commented 2 months ago

Thanks for logging the issue and the first attempts to replicate that in

I've moved your issue as there is an existing recipe in rewrite-testing-frameworks related to this issue https://github.com/openrewrite/rewrite-testing-frameworks/blob/17b0427564e5fa1d56b29f96469b7aea8c672210/src/main/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrows.java#L30-L42

Would you be able to adapt the example test there to fit the error seen here? https://github.com/openrewrite/rewrite-testing-frameworks/blob/17b0427564e5fa1d56b29f96469b7aea8c672210/src/test/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrowsTest.java#L73-L118

Ideally the test is stripped of everything not needed for replicate, so the initSomeTest for instance would not be needed.

timtebeek commented 2 months ago

Also might make sense to run through these steps to ensure your failure to see changes is not caused by missing types for instance.

FieteO commented 2 months ago

Just to be clear, is the ExpectedExceptionToAssertThrows recipe invoked as part of the SpringBoot2JUnit4to5Migration one?

timtebeek commented 2 months ago

Just to be clear, is the ExpectedExceptionToAssertThrows recipe invoked as part of the SpringBoot2JUnit4to5Migration one?

Yes! That's highlighted in green here and coming in through those two "hops". image

Malagutte commented 1 month ago
@Test(expected = ServiceException.class)
    public void myCustomTest() throws ServiceException {
        new Expectations() {{
            service.retrieveLocalizedValues(anyString);
            result = new ServiceException("Forced exception");
        }};

        masterdataService.retrieveLocalizedValues(DEFAULT_LANGUAGE);
    }

Do you think this code should be migrated using this recipe or should I use another one? Because right now, it is not

timtebeek commented 1 month ago

Do you think this code should be migrated using this recipe or should I use another one? Because right now, it is not

That ought to be converted by this recipe, as the expected exception is part of the annotation: https://docs.openrewrite.org/recipes/java/testing/junit5/updatetestannotation

That's different from the case reported by @FieteO above, as that uses a rule for the expected exception: https://docs.openrewrite.org/recipes/java/testing/junit5/expectedexceptiontoassertthrows

The steps to debug are the same, and outlined here: https://docs.openrewrite.org/reference/faq#my-recipe-runs-but-is-not-making-any-changes.-whats-happening

Additionally, since you're using JMockit, you might want to first and only run the JMockit to Mockito migration: https://docs.openrewrite.org/recipes/java/testing/jmockit/jmockittomockito