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:07 UTC

[maven-surefire] branch SUREFIRE-1669 updated (cd147f8 -> 95bfe70)

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

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


 discard cd147f8  [SUREFIRE-1669] POJO tests do not call fixture methods setUp and tearDown and test instance are not new between tests
     new a1617cd  [SUREFIRE-1669] POJO tests do not call fixture methods setUp and tearDown and test instance are not new between tests
     new 95bfe70  testSetCompleted has to be called anyway (finally block) in JUnit3Provider

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (cd147f8)
            \
             N -- N -- N   refs/heads/SUREFIRE-1669 (95bfe70)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:


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

Posted by ti...@apache.org.
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()
     {


[maven-surefire] 02/02: testSetCompleted has to be called anyway (finally block) in JUnit3Provider

Posted by ti...@apache.org.
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 95bfe70d026cbaf9f94931008513828afb508cc3
Author: tibordigana <ti...@apache.org>
AuthorDate: Mon May 27 10:22:35 2019 +0200

    testSetCompleted has to be called anyway (finally block) in JUnit3Provider
---
 .../apache/maven/surefire/junit/JUnit3Provider.java   | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnit3Provider.java b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnit3Provider.java
index d44d92b..e625888 100644
--- a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnit3Provider.java
+++ b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnit3Provider.java
@@ -28,6 +28,7 @@ import org.apache.maven.surefire.report.ConsoleOutputReceiver;
 import org.apache.maven.surefire.report.ReporterFactory;
 import org.apache.maven.surefire.report.RunListener;
 import org.apache.maven.surefire.report.SimpleReportEntry;
+import org.apache.maven.surefire.report.TestSetReportEntry;
 import org.apache.maven.surefire.suite.RunResult;
 import org.apache.maven.surefire.testset.TestSetFailedException;
 import org.apache.maven.surefire.util.ReflectionUtils;
@@ -112,7 +113,6 @@ public class JUnit3Provider
                 SurefireTestSet surefireTestSet = createTestSet( clazz );
                 executeTestSet( surefireTestSet, reporter, testClassLoader, systemProperties );
             }
-
         }
         finally
         {
@@ -122,7 +122,6 @@ public class JUnit3Provider
     }
 
     private SurefireTestSet createTestSet( Class<?> clazz )
-        throws TestSetFailedException
     {
         return reflector.isJUnit3Available() && jUnit3TestChecker.accept( clazz )
             ? new JUnitTestSet( clazz, reflector )
@@ -133,9 +132,19 @@ public class JUnit3Provider
                                  Map<String, String> systemProperties )
         throws TestSetFailedException
     {
-        reporter.testSetStarting( new SimpleReportEntry( testSet.getName(), null, null, null ) );
-        testSet.execute( reporter, classLoader );
-        reporter.testSetCompleted( new SimpleReportEntry( testSet.getName(), null, null, null, systemProperties ) );
+        String clazz = testSet.getName();
+
+        try
+        {
+            TestSetReportEntry started = new SimpleReportEntry( clazz, null, null, null );
+            reporter.testSetStarting( started );
+            testSet.execute( reporter, classLoader );
+        }
+        finally
+        {
+            TestSetReportEntry completed = new SimpleReportEntry( clazz, null, null, null, systemProperties );
+            reporter.testSetCompleted( completed );
+        }
     }
 
     private TestsToRun scanClassPath()