Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpringBoot2JUnit4to5Migration: JUnit4 assertions are not correctly updated to JUnit5. #171

Closed
josemariavillar opened this issue Mar 15, 2022 · 7 comments
Assignees

Comments

@josemariavillar
Copy link

Good afternoon, I am having problems when applying the SpringBoot2JUnit4to5Migration recipe in the following Test:

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

....

    @Test
    void assertionsTest() {
        assertNotNull(userRole.getId());
        assertEquals(role, userRole.getRole());
        assertEquals(user, userRole.getUser());

        Exception exception = assertThrows(NumberFormatException.class, new ThrowingRunnable() {
            public void run() throws Throwable {
                Integer.parseInt("1a");
            }
        });

        String expectedMessage = "For input string";
        String actualMessage = exception.getMessage();

        assertTrue(actualMessage.contains(expectedMessage));
        assertThat(actualMessage).isEqualTo(expectedMessage);
    }

When I run the mvn rewrite:dryRun or mvn rewrite:run command I would expect the "import static " to update correctly, instead the import of "assertEquals" and "assertNotNull" are removed and not replaced by their corresponding ones in Junit5 causing it to not compile the code, as can be seen in the following result:

--- "a/src\\test\\java\\com\\project\\service\\UserRoleTest.java"
+++ "b/src\\test\\java\\com\\project\\service\\UserRoleTest.java"
@@ -4,18 +4,16 @@ org.openrewrite.java.testing.junit5.AssertToAssertions, org.openrewrite.java.testing.junit5.UpdateBeforeAfterAnnotations, org.openrewrite.java.testing.junit5.UpdateTestAnnotation
 import com.project.model.User;
 import com.project.model.UserRole;
 import com.project.model.id.UserRoleKey;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.function.ThrowingRunnable;
+import org.junit.function.ThrowingRunnable;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 import java.sql.Timestamp;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class UserRoleTest {
 
@@ -28,7 +26,7 @@
     private final String CTE_CO_USER = "CTE_CO_USER";
     private final String CTE_CO_ROLE = "CTE_CO_ROLE";
 
-    @Before
+    @BeforeEach
     void setUp() {
         timestamp = Mockito.mock(Timestamp.class);

Attached is a test case where the problem can be reproduced and observed: https://github.com/josemariavillar/test_project/tree/assert_to_assertions

@pway99
Copy link
Contributor

pway99 commented Mar 15, 2022

Thanks @josemariavillar, I will have a look

@pway99 pway99 self-assigned this Mar 15, 2022
@pway99
Copy link
Contributor

pway99 commented Mar 15, 2022

@josemariavillar,
I could reproduce the issue; thanks so much for the test project.

FindMissingTypes is one of the first tools I use when investigating invalid recipe results. The results show several missing types. Looking at your project, I found that it's using Lombok, which is not supported at this time.

 user = /*~~(type is 'null')~~>*/User.builder().coUser(CTE_CO_USER)
                .coUserModification("codification")
                .tsModification(timestamp)
                .build();
        role = /*~~(type is 'null')~~>*/Role.builder().coRole(CTE_CO_ROLE)
                .coUserModification("codification")
                .tsModification(timestamp)
                .build();

        userRoleKey = /*~~(type is 'null')~~>*/UserRoleKey.builder().coUser(CTE_CO_USER).coRole(CTE_CO_ROLE)
                .build();

        userRole = /*~~(type is 'null')~~>*/UserRole.builder()
                .id(userRoleKey)
                .user(user)
                .role(role)
                .tsDelete(timestamp)
                .coUserModification("codification")
                .tsModification(timestamp)
                .build();
    }

    @Test
    void assertionsTest() {
        /*~~(type is 'null')~~>*/assertNotNull(userRole.getId());
        /*~~(type is 'null')~~>*/assertEquals(role, userRole.getRole());
        /*~~(type is 'null')~~>*/assertEquals(user, userRole.getUser());

@pway99 pway99 closed this as completed Mar 15, 2022
@pway99 pway99 moved this to Done in OpenRewrite Mar 15, 2022
@pway99
Copy link
Contributor

pway99 commented Mar 15, 2022

requires lombok support openrewrite/rewrite#1297

@josemariavillar
Copy link
Author

Thanks for the information @pway99. I have a doubt about what you comment, in this specific case what is affecting the lombok library? I don't understand it.

@josemariavillar
Copy link
Author

Regardless, a possible solution while supporting Loombok is to add generically the "import static org.junit.jupiter.api.Assertions.*", thus avoiding any problems.

@pway99
Copy link
Contributor

pway99 commented Mar 16, 2022

Hi @josemariavillar,

I have a doubt about what you comment, in this specific case what is affecting the lombok library? I don't understand it.

Thanks for asking, and please accept my apology for closing this issue with such a brief explanation.

In this specific case, the UserRole object uses Lombok annotations, so the UserRole.getId() method is not a member of the source file parsed by the rewrite-java-parser when the plugin is executed.

As a result the assertNotNull(userRole.getId()) statement has a null method type.

image

Consequently, the Assertions.assertNotNull import is not added by the AddImportVisitor because its type does not exist on the source tree.

@pway99
Copy link
Contributor

pway99 commented Mar 16, 2022

Regardless, a possible solution while supporting Lombok is to add generically the "import static org.junit.jupiter.api.Assertions.*", thus avoiding any problems.

Unfortunately, other recipes such as OrderImports might unfold the stared import, and the original problem remains.

Thanks again for identifying these issues :)

@rpau rpau added this to the Support Spring Boot 3 migrations milestone May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

3 participants