Recipe `org.openrewrite.java.testing.junit5.AssertToAssertions` breaks code with method with the same names as JUnit methods

[motlin]

What version of OpenRewrite are you using?

I am using

• rewrite-maven-plugin 5.30.0
• rewrite-testing-frameworks 2.8.0

What is the smallest, simplest way to reproduce the problem?

I'm running mvn --threads 1 -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.activeRecipes=org.openrewrite.java.testing.junit5.AssertToAssertions on Eclipse Collections as we prepare to move from JUnit 4 to 5. Eclipse Collections contains the test utility class Verify with methods that have similar signatures to Assert.

AssertToAssertions rearranges the parameters to Verify method calls, breaking the code.

-        Verify.assertNotContains(String.valueOf(90), actual1);
-        Verify.assertNotContains(String.valueOf(210), actual1);
-        Verify.assertContains(String.valueOf(159), actual1);
-        Assert.assertEquals(expected.getClass().getSimpleName() + '/' + actual1.getClass().getSimpleName(), expected, HashBag.newBag(actual1));
-        Assert.assertEquals(expected.getClass().getSimpleName() + '/' + actual2.getClass().getSimpleName(), expected, actual2);
-        Assert.assertEquals(expected.getClass().getSimpleName() + '/' + actual3.getClass().getSimpleName(), expected, actual3);
+        Verify.assertNotContains(actual1, String.valueOf(90));
+        Verify.assertNotContains(actual1, String.valueOf(210));
+        Verify.assertContains(actual1, String.valueOf(159));
+        Assertions.assertEquals(expected, HashBag.newBag(actual1), expected.getClass().getSimpleName() + '/' + actual1.getClass().getSimpleName());
+        Assertions.assertEquals(expected, actual2, expected.getClass().getSimpleName() + '/' + actual2.getClass().getSimpleName());
+        Assertions.assertEquals(expected, actual3, expected.getClass().getSimpleName() + '/' + actual3.getClass().getSimpleName());

[timtebeek]

Oh wow, that is quite unexpected; especially as we typically match on type quite strongly. In this particular case it seems we have a little more relaxed binding to accommodate common cases of missing type attribution, as of

• https://github.com/openrewrite/rewrite-testing-frameworks/pull/250 https://github.com/openrewrite/rewrite-testing-frameworks/blob/39dc67cab9a2d32ed000e5ad1348d31cae3979cd/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java#L131-L147

I wonder if that case is triggered here as well; does your actual use also use the Verify. prefix, or might that use static imports itself? Might be worth opening a draft PR with just a failing unit test if you're up for it as a step up to a fix. Either way thanks for the report!

[motlin]

My example comes from eclipse-collections-forkjoin/src/test/java/org/eclipse/collections/impl/forkjoin/FJIterateTest.java which does not use static imports for Verify.

[timtebeek]

Helpful context, thanks! And good to see the many improvements you're making there! :) Only just now noticed that Verify extends Assert; so I was able to replicate the issue with this unit test:

@Test
@Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/515")
void verifyClassExtendsAssertMethodArgumentsOrderRetained() {
    //language=java
    rewriteRun(
      java(
        """
          package foo;
          import org.junit.Assert;
          public class Verify extends Assert {
              public static void assertContains(String expected, String actual) {
              }
          }
          """,
        SourceSpec::skip
      ),
      java(
        """
          import foo.Verify;
          import org.junit.Assert;
          import java.util.List;

          class A {
              void test(String message, String expected, String actual) {
                  Verify.assertContains(expected, actual);
                  Assert.assertEquals(message, expected, actual);
              }
          }
          """,
        """
          import foo.Verify;
          import org.junit.jupiter.api.Assertions;

          import java.util.List;

          class A {
              void test(String message, String expected, String actual) {
                  Verify.assertContains(expected, actual);
                  Assertions.assertEquals(expected, actual, message);
              }
          }
          """
      )
    );
}

[timtebeek]

Looks to be fixed with https://github.com/openrewrite/rewrite-testing-frameworks/pull/516

• https://github.com/openrewrite/rewrite-testing-frameworks/pull/516

[timtebeek]

Ok; hope that gets you on your way again, as I saw there's lots of work to do. Let me know if we can help! :)

[motlin]

Thank you so much! I forgot that Verify extends Assert, which is an odd thing to do for static utility.

[timtebeek]

Yes was not expecting that extends, but once I'd spotted it the issue was easy to resolve. The fix should already be available from our snapshot versions; I expect to follow up with a release train to coincide with the Spring Boot 3.3 release this Thursday.

[timtebeek]

Also: if you're looking to try out various recipes on OSS then the Moderne CLI might be of interest to you: then you can build up the model once, serialize that, and use it for repeated recipe runs. That helps clear out the time consuming step of building up the model of your code that you see with just the OpenRewrite OSS plugins. The steps are roughly:

1. Install the CLI as per the getting started guide
2. Checkout and navigate to eclipse/eclipse-collections
3. Run mod config build maven arguments edit install --local=. as a quick workaround needed for this project
4. Run mod build . to build the LST once
5. Run mod run . --recipe=UseLambdaForFunctionalInterface to see the results in a couple seconds
6. Repeat step 5 for other recipes in our catalog

Each recipe run will produce a patch file that you can optionally mod git apply and then commit & push as normal. Figured that might help you iterate faster there.

Or you can run recipes and create pull requests directly through the Moderne Platform if you create a user organization with Eclipse collections: https://app.moderne.io/organizations/eclipse/eclipse-collections?branch=master&origin=github.com

Hope that helps!