Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SUREFIRE-2009] Refactoring of surefire-junit3. JUnitTestSetExecutor and PojoTestSetExecutor should be stateless. #467

Merged
merged 1 commit into from Feb 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -23,7 +23,6 @@
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;
Expand All @@ -37,6 +36,7 @@

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;
Expand All @@ -61,8 +61,6 @@ public class JUnit3Provider

private final ScanResult scanResult;

private TestsToRun testsToRun;

public JUnit3Provider( ProviderParameters booterParameters )
{
this.providerParameters = booterParameters;
Expand All @@ -78,35 +76,34 @@ public JUnit3Provider( ProviderParameters booterParameters )
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
Expand All @@ -131,14 +128,14 @@ static void setSystemManager( String smClassName ) throws TestSetFailedException
}
}

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
{
Expand All @@ -148,7 +145,7 @@ private void executeTestSet( SurefireTestSet testSet, RunListener reporter, Clas
{
TestSetReportEntry started = new SimpleReportEntry( clazz, null, null, null );
reporter.testSetStarting( started );
testSet.execute( reporter, classLoader );
testSetExecutor.execute( testSet, reporter, testClassLoader );
}
finally
{
Expand All @@ -159,14 +156,13 @@ private void executeTestSet( SurefireTestSet testSet, RunListener reporter, Clas

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();
}
}
Expand Up @@ -30,21 +30,13 @@
* 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;

// ----------------------------------------------------------------------
Expand All @@ -62,13 +54,10 @@ public JUnitTestSet( Class testClass, JUnit3Reflector reflector )
// 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 );
Expand Down Expand Up @@ -111,15 +100,4 @@ public void execute( RunListener reporter, ClassLoader loader )
throw new TestSetFailedException( testClass.getName(), e );
}
}

@Override
public String getName()
{
return testClass.getName();
}

private Class getTestClass()
{
return testClass;
}
}
Expand Up @@ -24,20 +24,22 @@
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";

Expand All @@ -47,57 +49,26 @@ 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;
}
}
}

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 )
Expand All @@ -116,16 +87,16 @@ private boolean executeTestMethod( Method method, RunListener reportManager )
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 )
{
Expand Down Expand Up @@ -164,7 +135,7 @@ private boolean executeTestMethod( Method method, RunListener reportManager )

try
{
tearDownFixture( testObject );
tearDownFixture( testObject, methods );
}
catch ( Throwable t )
{
Expand All @@ -188,54 +159,51 @@ private boolean executeTestMethod( Method method, RunListener reportManager )
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 )
Expand All @@ -251,14 +219,10 @@ private static boolean isValidTestMethod( Method m )
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;
}
}
Expand Up @@ -26,10 +26,8 @@
* 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();
}