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

[FieteO]

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]

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

• Add recipe to migrate JUnit 4's ExpectedException rewrite-spring#573

I've moved your issue as there is an existing recipe in rewrite-testing-frameworks related to this issue

rewrite-testing-frameworks/src/main/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrows.java

Lines 30 to 42 in 17b0427

	/**
	* Replace usages of JUnit 4's @Rule ExpectedException with JUnit 5 Assertions.
	* <p>
	* Supported ExpectedException methods:
	* expect(java.lang.Class)
	* expect(org.hamcrest.Matcher)
	* expectMessage(java.lang.String)
	* expectMessage(org.hamcrest.Matcher)
	* expectCause(org.hamcrest.Matcher)
	* <p>
	* Does not currently support migration of ExpectedException.isAnyExceptionExpected().
	*/
	public class ExpectedExceptionToAssertThrows extends Recipe {

Would you be able to adapt the example test there to fit the error seen here?

rewrite-testing-frameworks/src/test/java/org/openrewrite/java/testing/junit5/ExpectedExceptionToAssertThrowsTest.java

Lines 73 to 118 in 17b0427

	@DocumentExample
	@Test
	void expectedExceptionRule() {
	//language=java
	rewriteRun(
	java(
	"""
	import org.junit.Rule;
	import org.junit.Test;
	import org.junit.rules.ExpectedException;
	class MyTest {
	@Rule
	ExpectedException thrown = ExpectedException.none();
	@Test
	public void testEmptyPath() {
	this.thrown.expect(IllegalArgumentException.class);
	this.thrown.expectMessage("Invalid location: gs://");
	foo();
	}
	void foo() {
	}
	}
	""",
	"""
	import org.junit.Test;
	import static org.junit.jupiter.api.Assertions.assertThrows;
	import static org.junit.jupiter.api.Assertions.assertTrue;
	class MyTest {
	@Test
	public void testEmptyPath() {
	Throwable exception = assertThrows(IllegalArgumentException.class, () -> foo());
	assertTrue(exception.getMessage().contains("Invalid location: gs://"));
	}
	void foo() {
	}
	}
	"""
	)
	);
	}

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

[timtebeek]

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]

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

[timtebeek]

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".

[Malagutte]

@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]

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