Further improvements to replace powermockito method calls by mockito (#315)

* Unit Test for replacing mockStatic method.

* Modified recipe list

* Tried to use org.powermock for unit test

* Update src/test/java/org/openrewrite/java/testing/mockito/ReplacePowerMockitoIntegrationTest.java

Co-authored-by: Tim te Beek <[email protected]>

* Update src/test/java/org/openrewrite/java/testing/mockito/ReplacePowerMockitoIntegrationTest.java

* added unit test parser dependency to powermock

* Add powermock jars for classpathFromResources

* Removed additional Powermock annotations

* Exclude Visual Studio Code settings from git

* New Recipe PowerMockitoMockStaticToMockito

- First it removes the PowerMockTestCaseConfig extension

* Try to replace @PrepareForTest annotation with new MockedStatic fields

* Give the test method a meaningful name

* Mostly get the test cases passing. There is still a problem with the template which I think is a bug in OpenRewrite.

* Get tests passing

* Add missing license headers

* Updated mockito.yml and IntegrationTest to reflect changes from  last commit of code review

* Added tearDown() methods to the refactored tests

* Refactoring and code clean up

* Fixed issue with missing statements in tearDown method.

- Unit tests are failing because of missing type

* Tried to replace Mockito.when with MockedStatic.when

- Unit tests are failing because of missing type

* Specified requirements in ReplacePowerMockitoIntegrationTest

- Unit tests are failing because of missing type

* Fixed unit tests and 1 integration test

- The testNG case is still missing

- Unit tests are failing because of missing type

* First version of recipe to work with TestNG

* Refactor test framework information holder and fix unit test issues

* Enable ReplacePowerMockitoIntegrationTest and add testng-7.7.1 to classpathFromResources

* Fix annotation pattern in yml file

* Remove static inner class for annotation info and fix bugs

* Fix issue with parameterized method return types in when statement refactoring
* Fix issue when an unknown class is encountered while creating the MockedStatic fields

* Remove rewriting of when method calls

* New test for using PowerMockito without static import

* Improved mockedStatic method invocations

* Fixed NPE when visiting Interfaces or abstract classes

* Unit test for NPE when visiting Interfaces or abstract classes

* Fixed integration test

* Additional fixes

- Use TestNG as default test annotation
- Improve storing of mocked types to prevent mock not getting initialized
- Change name of generated setup method to prevent clashes
- Correctly parse used mocks from annotation with pattern @PrepareForTest(Class1.class)
- Add static import for Mockito.mockStatic even if "not used"
- Add back when method rewriting
- Rewrite PowerMock mock generation to use Mockito to prevent test runtime issues like ClassCastExceptions

* Code formatting

* Fixed all unit tests

* Refactoring to reduce the complexity

* Test powermockito calls are replaced in setUp methods

* Add parameters to @AfterMethod annotation

- replace close() with closeOnDemand()

* Additional tests for testng tearDown method

* Rearrange code

* Replace PowerMockito.spy on static methods

* Remove PowerMock extensions from test class

* Fix ArgumentException when staticMock returns an array[]

* Improved integration test

- still not working

* Fix issue with mockito static method not completely recognized

* Rewrite PowerMocktio.mockStatic with dynamic method calls

* Apply suggestions from code review

* Only create mocked field, if mockStatic is acutally called for the class

* Replace PowerMockito do-method calls with Mockito ones

* Reformat code; fix typo in mockito.yml

* Reformat code; Tests are now successful

* Fixed unit tests after merge of master

* Consider default implementations of interfaces

* Replaced instanceof in filter lambdas with class::isInstance

* Make use of visitors instead of parsing the class body

* Setup and teardown method must have same group as the test method

* Reduced cognitive complexity

---------

Co-authored-by: Tim te Beek <[email protected]>
Co-authored-by: Sam Snyder <[email protected]>
Co-authored-by: Tim te Beek <[email protected]>
Co-authored-by: D067517 <[email protected]>
Co-authored-by: Tobias Pöppke <[email protected]>

Author: 5 people

src/main/java/org/openrewrite/java/testing/.editorconfig

@@ -0,0 +1,5 @@
+root = true
+
+[*.java]
+indent_size = 4
+ij_continuation_indent_size = 2

src/main/java/org/openrewrite/java/testing/mockito/PowerMockitoMockStaticToMockito.java

@@ -171,5 +171,25 @@ recipeList:
       fullyQualifiedTargetTypeName: org.mockito.Mockito
       returnType: org.mockito.MockedStatic
       matchOverrides: null
+      matchUnknownTypes:
+  - org.openrewrite.java.ChangeMethodTargetToStatic:
+      methodPattern: org.powermock.api.mockito.PowerMockito do*(..)
+      fullyQualifiedTargetTypeName: org.mockito.Mockito
+      matchOverrides: null
+      matchUnknownTypes: null
+  - org.openrewrite.java.ChangeMethodTargetToStatic:
+      methodPattern: org.powermock.api.mockito.PowerMockito mock(..)
+      fullyQualifiedTargetTypeName: org.mockito.Mockito
+      matchOverrides: null
+      matchUnknownTypes: null
+  - org.openrewrite.java.ChangeMethodTargetToStatic:
+      methodPattern: org.powermock.api.mockito.PowerMockito spy(..)
+      fullyQualifiedTargetTypeName: org.mockito.Mockito
+      matchOverrides: null
+      matchUnknownTypes: null
+  - org.openrewrite.java.ChangeMethodTargetToStatic:
+      methodPattern: org.powermock.api.mockito.PowerMockito when(..)
+      fullyQualifiedTargetTypeName: org.mockito.Mockito
+      matchOverrides: null
       matchUnknownTypes: null
   - org.openrewrite.java.testing.mockito.PowerMockitoMockStaticToMockito

src/main/resources/META-INF/rewrite/classpath/powermock-api-mockito-1.6.5.jar

@@ -34,8 +34,9 @@ public void defaults(RecipeSpec spec) {
               "junit-4.13.2",
               "junit-jupiter-api-5.9.2",
               "mockito-core-3.12.4",
-              "powermock-api-mockito-1.7.4",
-              "powermock-core-1.7.4"
+              "powermock-api-mockito-1.6.5",
+              "powermock-core-1.6.5",
+              "testng-7.7.1"
             ))
           .recipe(new PowerMockitoMockStaticToMockito());
     }
@@ -77,13 +78,13 @@ public class MyTest {
                   private MockedStatic<Calendar> mockedCalendar;
               
                   @BeforeEach
-                  void setUp() {
+                  void setUpStaticMocks() {
                       mockedCalendar = mockStatic(Calendar.class);
                   }
               
                   @AfterEach
-                  void tearDown() {
-                      mockedCalendar.close();
+                  void tearDownStaticMocks() {
+                      mockedCalendar.closeOnDemand();
                   }
               
                   @Test
@@ -137,15 +138,15 @@ class MyTest {
                   private MockedStatic<Calendar> mockedCalendar;
               
                   @BeforeEach
-                  void setUp() {
-                      mockedCalendar = mockStatic(Calendar.class);
+                  void setUpStaticMocks() {
                       mockedCurrency = mockStatic(Currency.class);
+                      mockedCalendar = mockStatic(Calendar.class);
                   }
               
                   @AfterEach
-                  void tearDown() {
-                      mockedCalendar.close();
-                      mockedCurrency.close();
+                  void tearDownStaticMocks() {
+                      mockedCalendar.closeOnDemand();
+                      mockedCurrency.closeOnDemand();
                   }
               
                   @Test
@@ -198,8 +199,170 @@ void testStaticMethod() {
                   }
               
                   @AfterEach
-                  void tearDown() {
-                      mockedCalendar.close();
+                  void tearDownStaticMocks() {
+                      mockedCalendar.closeOnDemand();
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void tearDownMethodOfTestNGHasAnnotationWithArgument() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.util.Calendar;
+                            
+              import org.testng.annotations.Test;
+              import org.powermock.core.classloader.annotations.PrepareForTest;
+                            
+              @PrepareForTest({Calendar.class})
+              public class MyTest {
+                            
+                  @Test
+                  void testSomething() { }
+                    
+              }
+              """,
+            """
+              import java.util.Calendar;
+                            
+              import org.testng.annotations.AfterMethod;
+              import org.testng.annotations.BeforeMethod;
+              import org.testng.annotations.Test;
+                            
+              public class MyTest {
+
+                  @BeforeMethod
+                  void setUpStaticMocks() {
+                  }
+
+                  @AfterMethod(alwaysRun = true)
+                  void tearDownStaticMocks() {
+                  }
+
+                  @Test
+                  void testSomething() { }
+                            
+              }
+              """
+          )
+        );
+    }
+    @Test
+    void tearDownMethodOfTestNGWithAnnotationRemainsUntouched() {
+       //language=java
+        rewriteRun(
+          java(
+            """
+              import java.util.Calendar;
+                            
+              import org.testng.annotations.AfterMethod;
+              import org.testng.annotations.Test;
+              import org.powermock.core.classloader.annotations.PrepareForTest;
+                            
+              @PrepareForTest({Calendar.class})
+              public class MyTest {
+                            
+                  @AfterMethod(groups = "irrelevant")
+                  void tearDown() {}
+                  
+                  @Test
+                  void testSomething() { }
+                    
+              }
+              """,
+            """
+              import java.util.Calendar;
+                            
+              import org.testng.annotations.AfterMethod;
+              import org.testng.annotations.BeforeMethod;
+              import org.testng.annotations.Test;
+                            
+              public class MyTest {
+
+                  @AfterMethod(groups = "irrelevant")
+                  void tearDown() {}
+                  
+                  @BeforeMethod
+                  void setUpStaticMocks() {
+                  }
+
+                  @Test
+                  void testSomething() { }
+                            
+              }
+              """
+          )
+        );
+    }
+    @Test
+    void tearDownMethodOfTestNGHasAnnotationWithSameArgumentsAsTheTestThatCallsMockStatic() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.util.Calendar;
+                            
+              import static org.mockito.Mockito.*;
+                            
+              import org.testng.annotations.AfterMethod;
+              import org.testng.annotations.BeforeMethod;
+              import org.testng.annotations.Test;
+              import org.powermock.core.classloader.annotations.PrepareForTest;
+                            
+              @PrepareForTest({Calendar.class})
+              public class MyTest {
+                            
+                  private Calendar calendarMock;
+                  
+                  @Test(groups = "irrelevant")
+                  void testSomethingIrrelevantForCheckin() { }
+                  
+                  @Test(groups = "checkin")
+                  void testStaticMethod() {
+                      calendarMock = mock(Calendar.class);
+                      mockStatic(Calendar.class);
+                      when(Calendar.getInstance()).thenReturn(calendarMock);
+                  }
+              }
+              """,
+            """
+              import java.util.Calendar;
+                            
+              import static org.mockito.Mockito.*;
+                            
+              import org.mockito.MockedStatic;
+              import org.testng.annotations.AfterMethod;
+              import org.testng.annotations.BeforeMethod;
+              import org.testng.annotations.Test;
+
+              public class MyTest {
+
+                  private MockedStatic<Calendar> mockedCalendar;
+                  
+                  private Calendar calendarMock;
+
+                  @BeforeMethod(groups = "checkin")
+                  void setUpStaticMocks() {
+                      mockedCalendar = mockStatic(Calendar.class);
+                  }
+
+                  @AfterMethod(groups = "checkin")
+                  void tearDownStaticMocks() {
+                      mockedCalendar.closeOnDemand();
+                  }
+
+                  @Test(groups = "irrelevant")
+                  void testSomethingIrrelevantForCheckin() { }
+                  
+                  @Test(groups = "checkin")
+                  void testStaticMethod() {
+                      calendarMock = mock(Calendar.class);
+                      mockedCalendar.when(Calendar::getInstance).thenReturn(calendarMock);
                   }
               }
               """
@@ -284,6 +447,7 @@ void testStaticMethod() {
           )
         );
     }
+
     @Test
     void argumentOfVerifyOnParameterlessStaticMethodIsReplacedBySimpleLambda() {
         //language=java
@@ -312,29 +476,97 @@ void testStaticMethod() {
               }
               """,
             """
-             import static org.mockito.Mockito.*;
-                           
-             import java.util.Currency;
-             import java.util.Locale;
-                           
-             import org.junit.jupiter.api.Test;
-             import org.mockito.MockedStatic;
-                           
-             public class MyTest {
-                           
-                 private MockedStatic<Currency> mockedCurrency;
-               
-                 private Currency currencyMock = mock(Currency.class);
-                           
-                 @Test
-                 void testStaticMethod() {
-                     mockedCurrency.verify(() -> Currency.getInstance(Locale.ENGLISH), never());
-                     mockedCurrency.verify(Currency::getAvailableCurrencies, atLeastOnce());
-                 }
-             }
-             """
+               import static org.mockito.Mockito.*;
+                             
+               import java.util.Currency;
+               import java.util.Locale;
+                             
+               import org.junit.jupiter.api.Test;
+               import org.mockito.MockedStatic;
+                             
+               public class MyTest {
+                             
+                   private MockedStatic<Currency> mockedCurrency;
+                 
+                   private Currency currencyMock = mock(Currency.class);
+                             
+                   @Test
+                   void testStaticMethod() {
+                       mockedCurrency.verify(() -> Currency.getInstance(Locale.ENGLISH), never());
+                       mockedCurrency.verify(Currency::getAvailableCurrencies, atLeastOnce());
+                   }
+               }
+              """
           )
         );
     }
 
+    @Test
+    void interfacesAndAbstractClassesWithEmptyMethodBodiesRemainsUntouched() {
+        //language=java
+        rewriteRun(java(
+            """
+               public interface MyInterface {
+               
+                   void checkThis();
+               
+               }
+              """)
+          , java(
+            """ 
+              public abstract class MyAbstractClass {
+                          
+                  public boolean isItTrue() { return true; }
+                  
+                  public abstract boolean isItImplemented();
+                          
+              }
+              """
+          ));
+    }
+
+    @Test
+    void extensionOfPowerMockTestCaseGetsRemoved() {
+        //language=java
+        rewriteRun(java(
+            """
+              package org.powermock.modules.testng;
+
+              public class PowerMockTestCase {}
+              """
+          ),
+         java(
+            """
+              import org.powermock.modules.testng.PowerMockTestCase;
+
+              public class MyPowerMockTestCase extends PowerMockTestCase {}
+              """,
+            """
+              public class MyPowerMockTestCase {}
+              """)
+         );
+    }
+
+    @Test
+    void extensionOfPowerMockConfigurationGetsRemoved() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              package org.powermock.configuration;
+
+              public class PowerMockConfiguration {}
+              """
+          ),
+         java(
+            """
+              import org.powermock.configuration.PowerMockConfiguration;
+                
+              public class MyPowerMockConfiguration extends PowerMockConfiguration {}
+              """,
+            """
+              public class MyPowerMockConfiguration {}
+              """
+          ));
+    }
 }