From de0ffb7b98b92ea7214fc2125e3748d1fd1d803d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Pu=C5=A1k=C3=A1r?= Date: Wed, 19 Jul 2023 14:39:40 +0200 Subject: [PATCH 1/4] Add test for assertEquals edge case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - assertEquals in JUnit4 accepts Object[], to preserve the behaviour we need to rewrite it into assertArrayEquals Signed-off-by: Peter Puškár --- .../junit5/AssertToAssertionsTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java b/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java index 1f16e4f29..0f2ec5d1b 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java @@ -462,4 +462,35 @@ void test() { ) ); } + + // edge case for deprecated use of assertEquals + // https://junit.org/junit4/javadoc/4.13/org/junit/Assert.html#assertEquals(java.lang.Object%5B%5D,%20java.lang.Object%5B%5D) + @Issue("#383") + @Test + void assertEqualsWithArrayArgumentToAssertArrayEquals() { + //language=java + rewriteRun( + spec -> spec.typeValidationOptions(TypeValidation.none()), + java( + """ + import org.junit.Assert; + + class MyTest { + void test() { + Assert.assertEquals(new Object[1], new Object[1]); + } + } + """, + """ + import org.junit.jupiter.api.Assertions; + + class MyTest { + void test() { + Assertions.assertArrayEquals(new Object[1], new Object[1]); + } + } + """ + ) + ); + } } From f4fedb87ee6d3cd1f797d9ae5f9e768a5d50ba7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Pu=C5=A1k=C3=A1r?= Date: Thu, 20 Jul 2023 09:56:15 +0200 Subject: [PATCH 2/4] Address review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Puškár --- .../java/testing/junit5/AssertToAssertionsTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java b/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java index 0f2ec5d1b..bdc3464b6 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java @@ -465,12 +465,11 @@ void test() { // edge case for deprecated use of assertEquals // https://junit.org/junit4/javadoc/4.13/org/junit/Assert.html#assertEquals(java.lang.Object%5B%5D,%20java.lang.Object%5B%5D) - @Issue("#383") + @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/pull/384") @Test void assertEqualsWithArrayArgumentToAssertArrayEquals() { //language=java rewriteRun( - spec -> spec.typeValidationOptions(TypeValidation.none()), java( """ import org.junit.Assert; From 994442c4e360a8f8aa7ab8b2de7f518a0ada7a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Pu=C5=A1k=C3=A1r?= Date: Thu, 20 Jul 2023 10:00:39 +0200 Subject: [PATCH 3/4] Fix the issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Puškár --- .../java/testing/junit5/AssertToAssertions.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java b/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java index feb63f35a..991dc5492 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java @@ -81,6 +81,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu doAfterVisit(new ChangeMethodTargetToStatic("org.junit.Assert " + m.getSimpleName() + "(..)", "org.junit.jupiter.api.Assertions", null, null, true) .getVisitor()); + + if (isAssertEqualsWithArrayArguments(m)) { + // rename so we maintain the expected behaviour + m = m.withName(m.getName().withSimpleName("assertArrayEquals")); + } + List args = m.getArguments(); Expression firstArg = args.get(0); // Suppress arg-switching for Assertions.assertEquals(String, String) @@ -112,6 +118,15 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu return m; } + private static boolean isAssertEqualsWithArrayArguments(J.MethodInvocation method) { + if (!("assertEquals".equals(method.getSimpleName()))) { + return false; + } + // Detection based on 2nd argument which is the same in both overloads with and without message + Expression secondArg = method.getArguments().get(1); + return secondArg.getType() instanceof JavaType.Array; + } + private static boolean isJunitAssertMethod(J.MethodInvocation method) { if (method.getMethodType() != null && TypeUtils.isAssignableTo(ASSERTION_TYPE, method.getMethodType().getDeclaringType())) { return !"assertThat".equals(method.getSimpleName()); From 4f8741888bfe54565e7bdf87e2ea1609c2c71da6 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 20 Jul 2023 10:33:15 +0200 Subject: [PATCH 4/4] Move test and swap implementation to reuse ChangeMethodName --- .../testing/junit5/AssertToAssertions.java | 20 ++----------- .../resources/META-INF/rewrite/junit5.yml | 3 ++ .../junit5/AssertToAssertionsTest.java | 30 ------------------- .../testing/junit5/JUnit5MigrationTest.java | 30 ++++++++++++++++++- 4 files changed, 35 insertions(+), 48 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java b/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java index 991dc5492..ba134c75e 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/AssertToAssertions.java @@ -82,19 +82,14 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu "org.junit.jupiter.api.Assertions", null, null, true) .getVisitor()); - if (isAssertEqualsWithArrayArguments(m)) { - // rename so we maintain the expected behaviour - m = m.withName(m.getName().withSimpleName("assertArrayEquals")); - } - List args = m.getArguments(); Expression firstArg = args.get(0); // Suppress arg-switching for Assertions.assertEquals(String, String) if (args.size() == 2) { if ("assertSame".equals(m.getSimpleName()) || - "assertNotSame".equals(m.getSimpleName()) || - "assertEquals".equals(m.getSimpleName()) || - "assertNotEquals".equals(m.getSimpleName())) { + "assertNotSame".equals(m.getSimpleName()) || + "assertEquals".equals(m.getSimpleName()) || + "assertNotEquals".equals(m.getSimpleName())) { return m; } } @@ -118,15 +113,6 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu return m; } - private static boolean isAssertEqualsWithArrayArguments(J.MethodInvocation method) { - if (!("assertEquals".equals(method.getSimpleName()))) { - return false; - } - // Detection based on 2nd argument which is the same in both overloads with and without message - Expression secondArg = method.getArguments().get(1); - return secondArg.getType() instanceof JavaType.Array; - } - private static boolean isJunitAssertMethod(J.MethodInvocation method) { if (method.getMethodType() != null && TypeUtils.isAssignableTo(ASSERTION_TYPE, method.getMethodType().getDeclaringType())) { return !"assertThat".equals(method.getSimpleName()); diff --git a/src/main/resources/META-INF/rewrite/junit5.yml b/src/main/resources/META-INF/rewrite/junit5.yml index cec127f92..99e58adb9 100755 --- a/src/main/resources/META-INF/rewrite/junit5.yml +++ b/src/main/resources/META-INF/rewrite/junit5.yml @@ -65,6 +65,9 @@ recipeList: - org.openrewrite.java.testing.junit5.UseMockitoExtension - org.openrewrite.java.testing.junit5.UseTestMethodOrder - org.openrewrite.java.testing.junit5.MigrateJUnitTestCase + - org.openrewrite.java.ChangeMethodName: + methodPattern: "org.junit.Assert assertEquals(.., Object[], Object[])" + newMethodName: assertArrayEquals - org.openrewrite.java.testing.junit5.AssertToAssertions - org.openrewrite.java.testing.junit5.CategoryToTag - org.openrewrite.java.testing.junit5.CleanupJUnitImports diff --git a/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java b/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java index bdc3464b6..1f16e4f29 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/AssertToAssertionsTest.java @@ -462,34 +462,4 @@ void test() { ) ); } - - // edge case for deprecated use of assertEquals - // https://junit.org/junit4/javadoc/4.13/org/junit/Assert.html#assertEquals(java.lang.Object%5B%5D,%20java.lang.Object%5B%5D) - @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/pull/384") - @Test - void assertEqualsWithArrayArgumentToAssertArrayEquals() { - //language=java - rewriteRun( - java( - """ - import org.junit.Assert; - - class MyTest { - void test() { - Assert.assertEquals(new Object[1], new Object[1]); - } - } - """, - """ - import org.junit.jupiter.api.Assertions; - - class MyTest { - void test() { - Assertions.assertArrayEquals(new Object[1], new Object[1]); - } - } - """ - ) - ); - } } diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index 0750cc3c3..e3520de53 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -161,5 +161,33 @@ void upgradeMavenPluginVersions() { ); } - + // edge case for deprecated use of assertEquals + // https://junit.org/junit4/javadoc/4.13/org/junit/Assert.html#assertEquals(java.lang.Object%5B%5D,%20java.lang.Object%5B%5D) + @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/pull/384") + @Test + void assertEqualsWithArrayArgumentToAssertArrayEquals() { + //language=java + rewriteRun( + java( + """ + import org.junit.Assert; + + class MyTest { + void test() { + Assert.assertEquals(new Object[1], new Object[1]); + } + } + """, + """ + import org.junit.jupiter.api.Assertions; + + class MyTest { + void test() { + Assertions.assertArrayEquals(new Object[1], new Object[1]); + } + } + """ + ) + ); + } }