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 2022/02/12 09:56:14 UTC

[maven-surefire] branch test-run-id created (now 20f7a2f)

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

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


      at 20f7a2f  [SUREFIRE-2009] Refactoring of surefire-junit3. JUnitTestSetExecutor and PojoTestSetExecutor should be stateless.

This branch includes the following new commits:

     new 20f7a2f  [SUREFIRE-2009] Refactoring of surefire-junit3. JUnitTestSetExecutor and PojoTestSetExecutor should be stateless.

The 1 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.


[maven-surefire] 01/01: [SUREFIRE-2009] Refactoring of surefire-junit3. JUnitTestSetExecutor and PojoTestSetExecutor should be stateless.

Posted by ti...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 20f7a2fab42d9625bc67ed964550a1589788a24a
Author: tibor.digana <ti...@apache.org>
AuthorDate: Sat Feb 12 10:55:59 2022 +0100

    [SUREFIRE-2009] Refactoring of surefire-junit3. JUnitTestSetExecutor and PojoTestSetExecutor should be stateless.
---
 .../maven/surefire/junit/JUnit3Provider.java       |  20 ++--
 ...JUnitTestSet.java => JUnitTestSetExecutor.java} |  30 +-----
 .../{PojoTestSet.java => PojoTestSetExecutor.java} | 104 +++++++--------------
 ...reTestSet.java => SurefireTestSetExecutor.java} |   6 +-
 .../junit/TestListenerInvocationHandler.java       |  33 ++-----
 .../maven/surefire/junit/JUnitTestSetTest.java     |   4 +-
 6 files changed, 62 insertions(+), 135 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 ff210ec..731ac87 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
@@ -23,7 +23,6 @@ import org.apache.maven.surefire.common.junit3.JUnit3Reflector;
 import org.apache.maven.surefire.common.junit3.JUnit3TestChecker;
 import org.apache.maven.surefire.api.provider.AbstractProvider;
 import org.apache.maven.surefire.api.provider.ProviderParameters;
-import org.apache.maven.surefire.api.report.ConsoleOutputCapture;
 import org.apache.maven.surefire.api.report.ConsoleOutputReceiver;
 import org.apache.maven.surefire.api.report.ReporterFactory;
 import org.apache.maven.surefire.api.report.RunListener;
@@ -37,6 +36,7 @@ import org.apache.maven.surefire.api.util.TestsToRun;
 
 import java.util.Map;
 
+import static org.apache.maven.surefire.api.report.ConsoleOutputCapture.startCapture;
 import static org.apache.maven.surefire.api.util.ReflectionUtils.instantiate;
 import static org.apache.maven.surefire.api.util.internal.ObjectUtils.isSecurityManagerSupported;
 import static org.apache.maven.surefire.api.util.internal.ObjectUtils.systemProps;
@@ -98,15 +98,15 @@ public class JUnit3Provider
         RunResult runResult;
         try
         {
-            final RunListener reporter = reporterFactory.createReporter();
-            ConsoleOutputCapture.startCapture( (ConsoleOutputReceiver) reporter );
+            RunListener reporter = reporterFactory.createReporter();
+            startCapture( (ConsoleOutputReceiver) reporter );
             Map<String, String> systemProperties = systemProps();
             setSystemManager( System.getProperty( "surefire.security.manager" ) );
 
             for ( Class<?> clazz : testsToRun )
             {
-                SurefireTestSet surefireTestSet = createTestSet( clazz );
-                executeTestSet( surefireTestSet, reporter, testClassLoader, systemProperties );
+                SurefireTestSetExecutor surefireTestSetExecutor = createTestSet( clazz );
+                executeTestSet( clazz, surefireTestSetExecutor, reporter, systemProperties );
             }
         }
         finally
@@ -131,14 +131,14 @@ public class JUnit3Provider
         }
     }
 
-    private SurefireTestSet createTestSet( Class<?> clazz )
+    private SurefireTestSetExecutor createTestSet( Class<?> clazz )
     {
         return reflector.isJUnit3Available() && jUnit3TestChecker.accept( clazz )
-            ? new JUnitTestSet( clazz, reflector )
-            : new PojoTestSet( clazz );
+            ? new JUnitTestSetExecutor( reflector )
+            : new PojoTestSetExecutor();
     }
 
-    private void executeTestSet( SurefireTestSet testSet, RunListener reporter, ClassLoader classLoader,
+    private void executeTestSet( Class<?> testSet, SurefireTestSetExecutor testSetExecutor, RunListener reporter,
                                  Map<String, String> systemProperties )
         throws TestSetFailedException
     {
@@ -148,7 +148,7 @@ public class JUnit3Provider
         {
             TestSetReportEntry started = new SimpleReportEntry( clazz, null, null, null );
             reporter.testSetStarting( started );
-            testSet.execute( reporter, classLoader );
+            testSetExecutor.execute( testSet, reporter, testClassLoader );
         }
         finally
         {
diff --git a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnitTestSet.java b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnitTestSetExecutor.java
similarity index 85%
rename from surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnitTestSet.java
rename to surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnitTestSetExecutor.java
index 03ef5b0..e8b5d19 100644
--- a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnitTestSet.java
+++ b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/JUnitTestSetExecutor.java
@@ -30,21 +30,13 @@ import org.apache.maven.surefire.api.testset.TestSetFailedException;
  * JUnit3 test set
  *
  */
-public final class JUnitTestSet
-    implements SurefireTestSet
+public final class JUnitTestSetExecutor
+    implements SurefireTestSetExecutor
 {
-    private final Class testClass;
-
     private final JUnit3Reflector reflector;
 
-    public JUnitTestSet( Class testClass, JUnit3Reflector reflector )
+    public JUnitTestSetExecutor( JUnit3Reflector reflector )
     {
-        if ( testClass == null )
-        {
-            throw new NullPointerException( "testClass is null" );
-        }
-
-        this.testClass = testClass;
         this.reflector = reflector;
 
         // ----------------------------------------------------------------------
@@ -62,13 +54,10 @@ public final class JUnitTestSet
         // the same as the param types of TestResult.addTestListener
     }
 
-
     @Override
-    public void execute( RunListener reporter, ClassLoader loader )
+    public void execute( Class<?> testClass, RunListener reporter, ClassLoader loader )
         throws TestSetFailedException
     {
-        Class testClass = getTestClass();
-
         try
         {
             Object testObject = reflector.constructTestObject( testClass );
@@ -111,15 +100,4 @@ public final class JUnitTestSet
             throw new TestSetFailedException( testClass.getName(), e );
         }
     }
-
-    @Override
-    public String getName()
-    {
-        return testClass.getName();
-    }
-
-    private Class getTestClass()
-    {
-        return testClass;
-    }
 }
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/PojoTestSetExecutor.java
similarity index 73%
rename from surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/PojoTestSet.java
rename to surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/PojoTestSetExecutor.java
index 8dd6501..dcefba2 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/PojoTestSetExecutor.java
@@ -24,20 +24,22 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Collection;
+
 import org.apache.maven.surefire.api.report.LegacyPojoStackTraceWriter;
 import org.apache.maven.surefire.api.report.RunListener;
 import org.apache.maven.surefire.api.report.SimpleReportEntry;
 import org.apache.maven.surefire.api.report.StackTraceWriter;
 import org.apache.maven.surefire.api.testset.TestSetFailedException;
 
+import static java.util.Objects.requireNonNull;
 import static org.apache.maven.surefire.api.report.SimpleReportEntry.withException;
 
 /**
  * Executes a JUnit3 test class
  *
  */
-public class PojoTestSet
-    implements SurefireTestSet
+public class PojoTestSetExecutor
+    implements SurefireTestSetExecutor
 {
     private static final String TEST_METHOD_PREFIX = "test";
 
@@ -47,49 +49,17 @@ public class PojoTestSet
 
     private static final Object[] EMPTY_OBJECT_ARRAY = {};
 
-    private final Class<?> testClass;
-
-    private final Collection<Method> testMethods = new ArrayList<>();
-
-    private Method setUpMethod;
-
-    private Method tearDownMethod;
-
-    public PojoTestSet( final Class<?> testClass )
-    {
-        if ( testClass == null )
-        {
-            throw new IllegalArgumentException( "testClass is null" );
-        }
-
-        this.testClass = testClass;
-    }
-
     @Override
-    public void execute( RunListener reportManager, ClassLoader loader )
+    public void execute( Class<?> testClass, RunListener reportManager, ClassLoader loader )
             throws TestSetFailedException
     {
-        if ( reportManager == null )
-        {
-            throw new NullPointerException( "reportManager is null" );
-        }
+        requireNonNull( reportManager, "reportManager is null" );
 
-        executeTestMethods( reportManager );
-    }
+        DiscoveredTestMethods discoveredTestMethods = discoverTestMethods( testClass );
 
-    private void executeTestMethods( RunListener reportManager )
-            throws TestSetFailedException
-    {
-        if ( reportManager == null )
+        for ( Method testMethod : discoveredTestMethods.testMethods )
         {
-            throw new NullPointerException( "reportManager is null" );
-        }
-
-        discoverTestMethods();
-
-        for ( Method testMethod : testMethods )
-        {
-            boolean abort = executeTestMethod( testMethod, reportManager );
+            boolean abort = executeTestMethod( testClass, testMethod, discoveredTestMethods, reportManager );
             if ( abort )
             {
                 break;
@@ -97,7 +67,8 @@ public class PojoTestSet
         }
     }
 
-    private boolean executeTestMethod( Method method, RunListener reportManager )
+    private boolean executeTestMethod( Class<?> testClass, Method method, DiscoveredTestMethods methods,
+                                       RunListener reportManager )
             throws TestSetFailedException
     {
         if ( method == null || reportManager == null )
@@ -116,16 +87,16 @@ public class PojoTestSet
             throw new TestSetFailedException( "Unable to instantiate POJO '" + testClass + "'.", e );
         }
 
-        final String testClassName = getTestClass().getName();
+        final String testClassName = testClass.getName();
         final String methodName = method.getName();
         final String userFriendlyMethodName = methodName + "()";
-        final String testName = getTestName( userFriendlyMethodName );
+        final String testName = getTestName( testClassName, userFriendlyMethodName );
 
         reportManager.testStarting( new SimpleReportEntry( testClassName, null, testName, null ) );
 
         try
         {
-            setUpFixture( testObject );
+            setUpFixture( testObject, methods );
         }
         catch ( Throwable e )
         {
@@ -164,7 +135,7 @@ public class PojoTestSet
 
         try
         {
-            tearDownFixture( testObject );
+            tearDownFixture( testObject, methods );
         }
         catch ( Throwable t )
         {
@@ -188,54 +159,51 @@ public class PojoTestSet
         return false;
     }
 
-    private String getTestName( String testMethodName )
+    private String getTestName( String testClassName, String testMethodName )
     {
-        if ( testMethodName == null )
-        {
-            throw new NullPointerException( "testMethodName is null" );
-        }
-
-        return getTestClass().getName() + "." + testMethodName;
+        return testClassName + "." + requireNonNull( testMethodName, "testMethodName is null" );
     }
 
-    private void setUpFixture( Object testObject )
+    private void setUpFixture( Object testObject, DiscoveredTestMethods methods )
         throws Throwable
     {
-        if ( setUpMethod != null )
+        if ( methods.setUpMethod != null )
         {
-            setUpMethod.invoke( testObject );
+            methods.setUpMethod.invoke( testObject );
         }
     }
 
-    private void tearDownFixture( Object testObject )
+    private void tearDownFixture( Object testObject, DiscoveredTestMethods methods )
         throws Throwable
     {
-        if ( tearDownMethod != null )
+        if ( methods.tearDownMethod != null )
         {
-            tearDownMethod.invoke( testObject );
+            methods.tearDownMethod.invoke( testObject );
         }
     }
 
-    private void discoverTestMethods()
+    private DiscoveredTestMethods discoverTestMethods( Class<?> testClass )
     {
-        for ( Method m : getTestClass().getMethods() )
+        DiscoveredTestMethods methods = new DiscoveredTestMethods();
+        for ( Method m : testClass.getMethods() )
         {
             if ( isNoArgsInstanceMethod( m ) )
             {
                 if ( isValidTestMethod( m ) )
                 {
-                    testMethods.add( m );
+                    methods.testMethods.add( m );
                 }
                 else if ( SETUP_METHOD_NAME.equals( m.getName() ) )
                 {
-                    setUpMethod = m;
+                    methods.setUpMethod = m;
                 }
                 else if ( TEARDOWN_METHOD_NAME.equals( m.getName() ) )
                 {
-                    tearDownMethod = m;
+                    methods.tearDownMethod = m;
                 }
             }
         }
+        return methods;
     }
 
     private static boolean isNoArgsInstanceMethod( Method m )
@@ -251,14 +219,10 @@ public class PojoTestSet
         return m.getName().startsWith( TEST_METHOD_PREFIX );
     }
 
-    @Override
-    public String getName()
-    {
-        return getTestClass().getName();
-    }
-
-    private Class<?> getTestClass()
+    private static class DiscoveredTestMethods
     {
-        return testClass;
+        final Collection<Method> testMethods = new ArrayList<>();
+        Method setUpMethod;
+        Method tearDownMethod;
     }
 }
diff --git a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/SurefireTestSet.java b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/SurefireTestSetExecutor.java
similarity index 89%
rename from surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/SurefireTestSet.java
rename to surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/SurefireTestSetExecutor.java
index 4455bc9..e192081 100644
--- a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/SurefireTestSet.java
+++ b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/SurefireTestSetExecutor.java
@@ -26,10 +26,8 @@ import org.apache.maven.surefire.api.testset.TestSetFailedException;
  * Describes a single test set
  *
  */
-public interface SurefireTestSet
+public interface SurefireTestSetExecutor
 {
-    void execute( RunListener reportManager, ClassLoader loader )
+    void execute( Class<?> testClass, RunListener reportManager, ClassLoader loader )
         throws TestSetFailedException;
-
-    String getName();
 }
diff --git a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/TestListenerInvocationHandler.java b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/TestListenerInvocationHandler.java
index 219e064..cbbd655 100644
--- a/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/TestListenerInvocationHandler.java
+++ b/surefire-providers/surefire-junit3/src/main/java/org/apache/maven/surefire/junit/TestListenerInvocationHandler.java
@@ -30,6 +30,7 @@ import org.apache.maven.surefire.api.report.RunListener;
 import org.apache.maven.surefire.api.report.SimpleReportEntry;
 import org.apache.maven.surefire.api.report.StackTraceWriter;
 
+import static java.util.Objects.requireNonNull;
 import static org.apache.maven.surefire.api.report.SimpleReportEntry.withException;
 import static org.apache.maven.surefire.api.util.internal.TestClassMethodNameUtils.extractClassName;
 import static org.apache.maven.surefire.api.util.internal.TestClassMethodNameUtils.extractMethodName;
@@ -52,33 +53,24 @@ public class TestListenerInvocationHandler
 
     private final Set<FailedTest> failedTestsSet = new HashSet<>();
 
-    private RunListener reporter;
+    private final RunListener reporter;
 
-    private static final Class[] EMPTY_CLASS_ARRAY = { };
+    private static final Class<?>[] EMPTY_CLASS_ARRAY = { };
 
     private static final Object[] EMPTY_STRING_ARRAY = { };
 
     private static class FailedTest
     {
-        private Object testThatFailed;
-
-        private Thread threadOnWhichTestFailed;
+        private final Object testThatFailed;
+        private final Thread threadOnWhichTestFailed;
 
         FailedTest( Object testThatFailed, Thread threadOnWhichTestFailed )
         {
-            if ( testThatFailed == null )
-            {
-                throw new NullPointerException( "testThatFailed is null" );
-            }
-
-            if ( threadOnWhichTestFailed == null )
-            {
-                throw new NullPointerException( "threadOnWhichTestFailed is null" );
-            }
-
-            this.testThatFailed = testThatFailed;
+            this.testThatFailed =
+                requireNonNull( testThatFailed, "testThatFailed is null" );
 
-            this.threadOnWhichTestFailed = threadOnWhichTestFailed;
+            this.threadOnWhichTestFailed =
+                requireNonNull( threadOnWhichTestFailed, "threadOnWhichTestFailed is null" );
         }
 
         @Override
@@ -116,12 +108,7 @@ public class TestListenerInvocationHandler
 
     public TestListenerInvocationHandler( RunListener reporter )
     {
-        if ( reporter == null )
-        {
-            throw new NullPointerException( "reporter is null" );
-        }
-
-        this.reporter = reporter;
+        this.reporter = requireNonNull( reporter, "reporter is null" );
     }
 
     @Override
diff --git a/surefire-providers/surefire-junit3/src/test/java/org/apache/maven/surefire/junit/JUnitTestSetTest.java b/surefire-providers/surefire-junit3/src/test/java/org/apache/maven/surefire/junit/JUnitTestSetTest.java
index 67d1ece..0a0e1c1 100644
--- a/surefire-providers/surefire-junit3/src/test/java/org/apache/maven/surefire/junit/JUnitTestSetTest.java
+++ b/surefire-providers/surefire-junit3/src/test/java/org/apache/maven/surefire/junit/JUnitTestSetTest.java
@@ -50,9 +50,9 @@ public class JUnitTestSetTest
     {
         ClassLoader testClassLoader = this.getClass().getClassLoader();
         JUnit3Reflector reflector = new JUnit3Reflector( testClassLoader );
-        JUnitTestSet testSet = new JUnitTestSet( Suite.class, reflector );
+        JUnitTestSetExecutor testSet = new JUnitTestSetExecutor( reflector );
         SuccessListener listener = new SuccessListener();
-        testSet.execute( listener, testClassLoader );
+        testSet.execute( Suite.class, listener, testClassLoader );
         List<ReportEntry> succeededTests = listener.getSucceededTests();
         assertEquals( 1, succeededTests.size() );
         assertEquals( "org.apache.maven.surefire.junit.JUnitTestSetTest$AlwaysSucceeds",