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 <tibordig...@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",