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 17:22:33 UTC
[maven-surefire] branch master updated: [SUREFIRE-2009] Refactoring of surefire-junit3. JUnitTestSetExecutor and PojoTestSetExecutor should be stateless.
This is an automated email from the ASF dual-hosted git repository.
tibordigana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
The following commit(s) were added to refs/heads/master by this push:
new 93e0aea [SUREFIRE-2009] Refactoring of surefire-junit3. JUnitTestSetExecutor and PojoTestSetExecutor should be stateless.
93e0aea is described below
commit 93e0aead64d290fc241924a7fc323b49b61b55c9
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 | 52 +++++------
...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, 76 insertions(+), 153 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..3c9825d 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;
@@ -61,8 +61,6 @@ public class JUnit3Provider
private final ScanResult scanResult;
- private TestsToRun testsToRun;
-
public JUnit3Provider( ProviderParameters booterParameters )
{
this.providerParameters = booterParameters;
@@ -78,35 +76,34 @@ public class JUnit3Provider
public RunResult invoke( Object forkTestSet )
throws TestSetFailedException
{
- if ( testsToRun == null )
+
+ final TestsToRun testsToRun;
+ if ( forkTestSet instanceof TestsToRun )
{
- if ( forkTestSet instanceof TestsToRun )
- {
- testsToRun = (TestsToRun) forkTestSet;
- }
- else if ( forkTestSet instanceof Class )
- {
- testsToRun = TestsToRun.fromClass( (Class<?>) forkTestSet );
- }
- else
- {
- testsToRun = scanClassPath();
- }
+ testsToRun = (TestsToRun) forkTestSet;
+ }
+ else if ( forkTestSet instanceof Class )
+ {
+ testsToRun = TestsToRun.fromClass( (Class<?>) forkTestSet );
+ }
+ else
+ {
+ testsToRun = scanClassPath();
}
ReporterFactory reporterFactory = providerParameters.getReporterFactory();
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 +128,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 +145,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
{
@@ -159,14 +156,13 @@ public class JUnit3Provider
private TestsToRun scanClassPath()
{
- final TestsToRun testsToRun = scanResult.applyFilter( testChecker, testClassLoader );
+ TestsToRun testsToRun = scanResult.applyFilter( testChecker, testClassLoader );
return runOrderCalculator.orderTestClasses( testsToRun );
}
@Override
public Iterable<Class<?>> getSuites()
{
- testsToRun = scanClassPath();
- return testsToRun;
+ return scanClassPath();
}
}
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",