You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by ti...@apache.org on 2019/05/27 08:23:08 UTC

[maven-surefire] 01/02: [SUREFIRE-1669] POJO tests do not call fixture methods setUp and tearDown and test instance are not new between tests

This is an automated email from the ASF dual-hosted git repository.

tibordigana pushed a commit to branch SUREFIRE-1669
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git

commit a1617cd201d7baf00fc636a45b1d142e922f0856
Author: tibordigana <ti...@apache.org>
AuthorDate: Mon May 27 09:01:48 2019 +0200

    [SUREFIRE-1669] POJO tests do not call fixture methods setUp and tearDown and test instance are not new between tests
---
 .../apache/maven/surefire/its/PojoSimpleIT.java    |  14 ++-
 .../pojo-simple/src/test/java/PojoTest.java        |  11 +++
 .../apache/maven/surefire/junit/PojoTestSet.java   | 101 ++++++++++-----------
 3 files changed, 69 insertions(+), 57 deletions(-)

diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/PojoSimpleIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/PojoSimpleIT.java
index 03902ef..6905730 100644
--- a/surefire-its/src/test/java/org/apache/maven/surefire/its/PojoSimpleIT.java
+++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/PojoSimpleIT.java
@@ -19,9 +19,13 @@ package org.apache.maven.surefire.its;
  * under the License.
  */
 
+import org.apache.maven.it.VerificationException;
 import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase;
 import org.junit.Test;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+
 /**
  * Test support for POJO tests.
  *
@@ -31,8 +35,14 @@ public class PojoSimpleIT
     extends SurefireJUnit4IntegrationTestCase
 {
     @Test
-    public void testit()
+    public void twoTestsWithFixtures() throws VerificationException
     {
-        unpack( "pojo-simple" ).executeTest().assertTestSuiteResults( 2, 0, 1, 0 );
+        unpack( "pojo-simple" )
+                .executeTest()
+                .assertTestSuiteResults( 2, 0, 1, 0 )
+                .assertThatLogLine( containsString( "setUp called 1" ), is( 1 ) )
+                .assertThatLogLine( containsString( "setUp called 2" ), is( 1 ) )
+                .assertThatLogLine( containsString( "tearDown called 1" ), is( 1 ) )
+                .assertThatLogLine( containsString( "tearDown called 2" ), is( 1 ) );
     }
 }
diff --git a/surefire-its/src/test/resources/pojo-simple/src/test/java/PojoTest.java b/surefire-its/src/test/resources/pojo-simple/src/test/java/PojoTest.java
index 8e13ecb..18fad81 100644
--- a/surefire-its/src/test/resources/pojo-simple/src/test/java/PojoTest.java
+++ b/surefire-its/src/test/resources/pojo-simple/src/test/java/PojoTest.java
@@ -19,6 +19,17 @@
 
 public class PojoTest
 {
+    private static int calls;
+
+    public void setUp()
+    {
+        System.out.println( "setUp called " + ++calls );
+    }
+
+    public void tearDown()
+    {
+        System.out.println( "tearDown called " + calls );
+    }
 
     public void testSuccess()
     {
diff --git a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/PojoTestSet.java b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/PojoTestSet.java
index 7ee787e..d0dab06 100644
--- a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/PojoTestSet.java
+++ b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/PojoTestSet.java
@@ -23,7 +23,7 @@ import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
-import java.util.List;
+import java.util.Collection;
 import org.apache.maven.surefire.report.LegacyPojoStackTraceWriter;
 import org.apache.maven.surefire.report.RunListener;
 import org.apache.maven.surefire.report.SimpleReportEntry;
@@ -41,20 +41,21 @@ public class PojoTestSet
 {
     private static final String TEST_METHOD_PREFIX = "test";
 
-    private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0];
+    private static final String SETUP_METHOD_NAME = "setUp";
 
-    private final Object testObject;
+    private static final String TEARDOWN_METHOD_NAME = "tearDown";
+
+    private static final Object[] EMPTY_OBJECT_ARRAY = {};
 
     private final Class<?> testClass;
 
-    private List<Method> testMethods;
+    private final Collection<Method> testMethods = new ArrayList<>();
 
     private Method setUpMethod;
 
     private Method tearDownMethod;
 
     public PojoTestSet( final Class<?> testClass )
-        throws TestSetFailedException
     {
         if ( testClass == null )
         {
@@ -62,19 +63,11 @@ public class PojoTestSet
         }
 
         this.testClass = testClass;
-
-        try
-        {
-            testObject = testClass.newInstance();
-        }
-        catch ( ReflectiveOperationException e )
-        {
-            throw new TestSetFailedException( "Unable to instantiate POJO '" + testClass + "'", e );
-        }
     }
 
     @Override
     public void execute( RunListener reportManager, ClassLoader loader )
+            throws TestSetFailedException
     {
         if ( reportManager == null )
         {
@@ -85,42 +78,54 @@ public class PojoTestSet
     }
 
     private void executeTestMethods( RunListener reportManager )
+            throws TestSetFailedException
     {
         if ( reportManager == null )
         {
             throw new NullPointerException( "reportManager is null" );
         }
 
-        if ( testMethods == null )
-        {
-            discoverTestMethods();
-        }
+        discoverTestMethods();
 
-        boolean abort = false;
-
-        for ( int i = 0; i < testMethods.size() && !abort; ++i )
+        for ( Method testMethod : testMethods )
         {
-            abort = executeTestMethod( testMethods.get( i ), EMPTY_OBJECT_ARRAY, reportManager );
+            boolean abort = executeTestMethod( testMethod, reportManager );
+            if ( abort )
+            {
+                break;
+            }
         }
     }
 
-    private boolean executeTestMethod( Method method, Object[] args, RunListener reportManager )
+    private boolean executeTestMethod( Method method, RunListener reportManager )
+            throws TestSetFailedException
     {
-        if ( method == null || args == null || reportManager == null )
+        if ( method == null || reportManager == null )
         {
             throw new NullPointerException();
         }
 
+        final Object testObject;
+
+        try
+        {
+            testObject = testClass.newInstance();
+        }
+        catch ( ReflectiveOperationException e )
+        {
+            throw new TestSetFailedException( "Unable to instantiate POJO '" + testClass + "'.", e );
+        }
+
         final String testClassName = getTestClass().getName();
         final String methodName = method.getName();
-        final String userFriendlyMethodName = methodName + '(' + ( args.length == 0 ? "" : "Reporter" ) + ')';
+        final String userFriendlyMethodName = methodName + "()";
         final String testName = getTestName( userFriendlyMethodName );
 
         reportManager.testStarting( new SimpleReportEntry( testClassName, null, testName, null ) );
 
         try
         {
-            setUpFixture();
+            setUpFixture( testObject );
         }
         catch ( Throwable e )
         {
@@ -138,7 +143,7 @@ public class PojoTestSet
         // Make sure that tearDownFixture
         try
         {
-            method.invoke( testObject, args );
+            method.invoke( testObject, EMPTY_OBJECT_ARRAY );
             reportManager.testSucceeded( new SimpleReportEntry( testClassName, null, testName, null ) );
         }
         catch ( InvocationTargetException e )
@@ -159,7 +164,7 @@ public class PojoTestSet
 
         try
         {
-            tearDownFixture();
+            tearDownFixture( testObject );
         }
         catch ( Throwable t )
         {
@@ -193,7 +198,7 @@ public class PojoTestSet
         return getTestClass().getName() + "." + testMethodName;
     }
 
-    private void setUpFixture()
+    private void setUpFixture( Object testObject )
         throws Throwable
     {
         if ( setUpMethod != null )
@@ -202,7 +207,7 @@ public class PojoTestSet
         }
     }
 
-    private void tearDownFixture()
+    private void tearDownFixture( Object testObject )
         throws Throwable
     {
         if ( tearDownMethod != null )
@@ -213,35 +218,19 @@ public class PojoTestSet
 
     private void discoverTestMethods()
     {
-        if ( testMethods == null )
+        for ( Method m : getTestClass().getMethods() )
         {
-            testMethods = new ArrayList<>();
-
-            Method[] methods = getTestClass().getMethods();
-
-            for ( Method m : methods )
+            if ( isNoArgsInstanceMethod( m ) )
             {
                 if ( isValidTestMethod( m ) )
                 {
-                    String simpleName = m.getName();
-
-                    // name must have 5 or more chars
-                    if ( simpleName.length() > 4 )
-                    {
-                        String firstFour = simpleName.substring( 0, 4 );
-
-                        // name must start with "test"
-                        if ( firstFour.equals( TEST_METHOD_PREFIX ) )
-                        {
-                            testMethods.add( m );
-                        }
-                    }
+                    testMethods.add( m );
                 }
-                else if ( m.getName().equals( "setUp" ) && m.getParameterTypes().length == 0 )
+                else if ( SETUP_METHOD_NAME.equals( m.getName() ) )
                 {
                     setUpMethod = m;
                 }
-                else if ( m.getName().equals( "tearDown" ) && m.getParameterTypes().length == 0 )
+                else if ( TEARDOWN_METHOD_NAME.equals( m.getName() ) )
                 {
                     tearDownMethod = m;
                 }
@@ -249,17 +238,19 @@ public class PojoTestSet
         }
     }
 
-    private static boolean isValidTestMethod( Method m )
+    private static boolean isNoArgsInstanceMethod( Method m )
     {
         boolean isInstanceMethod = !Modifier.isStatic( m.getModifiers() );
-
         boolean returnsVoid = m.getReturnType().equals( void.class );
-
         boolean hasNoParams = m.getParameterTypes().length == 0;
-
         return isInstanceMethod && returnsVoid && hasNoParams;
     }
 
+    private static boolean isValidTestMethod( Method m )
+    {
+        return m.getName().startsWith( TEST_METHOD_PREFIX );
+    }
+
     @Override
     public String getName()
     {