From 769626abe5d01ac6d5b8e4f8cd01191734b8c93c Mon Sep 17 00:00:00 2001 From: agudian Date: Sat, 8 Dec 2012 21:02:02 +0100 Subject: [PATCH 1/2] [SUREFIRE-839] Tests not matching categories would fail build Fixed in a way that does not break forkMode=onceperthread (enhanced the IT for that), refined the logic of JUnitCoreProvider.canRunClass (to also detect the BasicTest class from the IT) --- .../Surefire839TestWithoutCategoriesIT.java | 6 +++ .../surefire/junitcore/JUnitCoreProvider.java | 49 +++++-------------- .../surefire/junitcore/JUnitCoreWrapper.java | 18 ++++--- 3 files changed, 31 insertions(+), 42 deletions(-) diff --git a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java index 6c0334928a..5fe25d5150 100755 --- a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java +++ b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java @@ -30,4 +30,10 @@ public void classWithoutCategory() { unpack( "junit48-categories" ).setJUnitVersion( "4.11" ).executeTest().verifyErrorFree( 2 ); } + + @Test + public void classWithoutCategoryForked() + { + unpack( "junit48-categories" ).setJUnitVersion( "4.11" ).forkOncePerThread().threadCount(2).executeTest().verifyErrorFree( 2 ); + } } diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java index 81ea524153..908ffb3451 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java @@ -103,22 +103,6 @@ public Iterator getSuites() return testsToRun.iterator(); } - private boolean containsSomethingRunnable( TestsToRun testsToRun, Filter filter ) - { - if ( filter == null ) - { - return true; - } - for ( Class o : testsToRun.getLocatedClasses() ) - { - if ( canRunClass( filter, o ) ) - { - return true; - } - } - return false; - } - public RunResult invoke( Object forkTestSet ) throws TestSetFailedException, ReporterException { @@ -147,24 +131,20 @@ else if ( forkTestSet instanceof Class ) } } - if ( containsSomethingRunnable( testsToRun, filter ) ) - { - - final Map testSetMap = new ConcurrentHashMap(); + final Map testSetMap = new ConcurrentHashMap(); - RunListener listener = ConcurrentReporterManager.createInstance( testSetMap, reporterFactory, - jUnitCoreParameters.isParallelClasses(), - jUnitCoreParameters.isParallelBoth(), - consoleLogger ); + RunListener listener = ConcurrentReporterManager.createInstance( testSetMap, reporterFactory, + jUnitCoreParameters.isParallelClasses(), + jUnitCoreParameters.isParallelBoth(), + consoleLogger ); - ConsoleOutputCapture.startCapture( (ConsoleOutputReceiver) listener ); + ConsoleOutputCapture.startCapture( (ConsoleOutputReceiver) listener ); - org.junit.runner.notification.RunListener jUnit4RunListener = - new JUnitCoreRunListener( listener, testSetMap ); - customRunListeners.add( 0, jUnit4RunListener ); + org.junit.runner.notification.RunListener jUnit4RunListener = + new JUnitCoreRunListener( listener, testSetMap ); + customRunListeners.add( 0, jUnit4RunListener ); - JUnitCoreWrapper.execute( testsToRun, jUnitCoreParameters, customRunListeners, filter ); - } + JUnitCoreWrapper.execute( testsToRun, jUnitCoreParameters, customRunListeners, filter ); return reporterFactory.close(); } @@ -192,14 +172,10 @@ private TestsToRun getSuitesAsList( Filter filter ) private boolean canRunClass( Filter filter, Class clazz ) { - boolean isCategoryAnnotatedClass = jUnit48Reflector.isCategoryAnnotationPresent( clazz ); - Description d = Description.createSuiteDescription( clazz ); + final Description d = Description.createSuiteDescription( clazz ); if ( filter.shouldRun( d ) ) { - return true; - } - else - { + // if the class-level check passes, we need to check if any methods are left to run for ( Method method : clazz.getMethods() ) { final Description testDescription = @@ -210,6 +186,7 @@ private boolean canRunClass( Filter filter, Class clazz ) } } } + return false; } diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java index aeda52398f..1110679c0d 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java @@ -19,6 +19,7 @@ * under the License. */ +import java.util.Iterator; import java.util.List; import java.util.concurrent.ExecutionException; import org.apache.maven.surefire.common.junit4.JUnit4RunListener; @@ -53,14 +54,19 @@ public static void execute( TestsToRun testsToRun, JUnitCoreParameters jUnitCore try { - Request req = Request.classes( computer, testsToRun.getLocatedClasses() ); - if ( filter != null ) + // in order to support LazyTestsToRun, the iterator must be used + Iterator classIter = testsToRun.iterator(); + while (classIter.hasNext()) { - req = req.filterWith( filter ); - } + Request req = Request.classes( computer, new Class[]{ (Class) classIter.next() }); + if ( filter != null ) + { + req = req.filterWith( filter ); + } - final Result run = junitCore.run( req ); - JUnit4RunListener.rethrowAnyTestMechanismFailures( run ); + final Result run = junitCore.run( req ); + JUnit4RunListener.rethrowAnyTestMechanismFailures( run ); + } } finally { From 6a70e29e2fb7f55ab69af4e1a3ff8d8d32314117 Mon Sep 17 00:00:00 2001 From: agudian Date: Sat, 8 Dec 2012 23:30:11 +0100 Subject: [PATCH 2/2] [SUREFIRE-839] Tests not matching categories would fail build Tests are now filtered right before handing them over to JUnit. In case of a NoTestsRemainException while filtering the test request, nothing will be handed over to JUnit. --- .../Surefire839TestWithoutCategoriesIT.java | 4 +- .../surefire/junitcore/JUnitCoreProvider.java | 46 +------------------ .../surefire/junitcore/JUnitCoreWrapper.java | 29 +++++++++++- 3 files changed, 31 insertions(+), 48 deletions(-) diff --git a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java index 5fe25d5150..03a715cf82 100755 --- a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java +++ b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire839TestWithoutCategoriesIT.java @@ -28,12 +28,12 @@ public class Surefire839TestWithoutCategoriesIT @Test public void classWithoutCategory() { - unpack( "junit48-categories" ).setJUnitVersion( "4.11" ).executeTest().verifyErrorFree( 2 ); + unpack( "junit48-categories" ).setJUnitVersion( "4.11" ).executeTest().verifyErrorFree( 3 ); } @Test public void classWithoutCategoryForked() { - unpack( "junit48-categories" ).setJUnitVersion( "4.11" ).forkOncePerThread().threadCount(2).executeTest().verifyErrorFree( 2 ); + unpack( "junit48-categories" ).setJUnitVersion( "4.11" ).forkOncePerThread().threadCount( 2 ).executeTest().verifyErrorFree( 3 ); } } diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java index 908ffb3451..94c19e7cee 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java @@ -99,7 +99,7 @@ public Boolean isRunnable() public Iterator getSuites() { final Filter filter = jUnit48Reflector.isJUnit48Available() ? createJUnit48Filter() : null; - testsToRun = getSuitesAsList( filter ); + testsToRun = scanClassPath(); return testsToRun.iterator(); } @@ -127,7 +127,7 @@ else if ( forkTestSet instanceof Class ) } else { - testsToRun = getSuitesAsList( filter ); + testsToRun = scanClassPath(); } } @@ -148,48 +148,6 @@ else if ( forkTestSet instanceof Class ) return reporterFactory.close(); } - @SuppressWarnings( "unchecked" ) - private TestsToRun getSuitesAsList( Filter filter ) - { - List> res = new ArrayList>( 500 ); - TestsToRun max = scanClassPath(); - if ( filter == null ) - { - return max; - } - - Iterator> it = max.iterator(); - while ( it.hasNext() ) - { - Class clazz = it.next(); - if ( canRunClass( filter, clazz ) ) - { - res.add( clazz ); - } - } - return new TestsToRun( res ); - } - - private boolean canRunClass( Filter filter, Class clazz ) - { - final Description d = Description.createSuiteDescription( clazz ); - if ( filter.shouldRun( d ) ) - { - // if the class-level check passes, we need to check if any methods are left to run - for ( Method method : clazz.getMethods() ) - { - final Description testDescription = - Description.createTestDescription( clazz, method.getName(), method.getAnnotations() ); - if ( filter.shouldRun( testDescription ) ) - { - return true; - } - } - } - - return false; - } - private Filter createJUnit48Filter() { final FilterFactory filterFactory = new FilterFactory( testClassLoader ); diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java index 1110679c0d..fea64f74a2 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreWrapper.java @@ -30,7 +30,9 @@ import org.junit.runner.JUnitCore; import org.junit.runner.Request; import org.junit.runner.Result; +import org.junit.runner.Runner; import org.junit.runner.manipulation.Filter; +import org.junit.runner.manipulation.NoTestsRemainException; import org.junit.runner.notification.RunListener; /** @@ -41,6 +43,25 @@ class JUnitCoreWrapper { + private static class FilteringRequest extends Request { + private Runner filteredRunner; + + public FilteringRequest( Request req, Filter filter ) { + try { + Runner runner= req.getRunner(); + filter.apply( runner ); + filteredRunner = runner; + } catch ( NoTestsRemainException e ) { + filteredRunner = null; + } + } + + @Override + public Runner getRunner() { + return filteredRunner; + } + } + public static void execute( TestsToRun testsToRun, JUnitCoreParameters jUnitCoreParameters, List listeners, Filter filter ) throws TestSetFailedException @@ -61,7 +82,11 @@ public static void execute( TestsToRun testsToRun, JUnitCoreParameters jUnitCore Request req = Request.classes( computer, new Class[]{ (Class) classIter.next() }); if ( filter != null ) { - req = req.filterWith( filter ); + req = new FilteringRequest( req, filter ); + if ( req.getRunner() == null ) + { + continue; + } } final Result run = junitCore.run( req ); @@ -77,7 +102,7 @@ public static void execute( TestsToRun testsToRun, JUnitCoreParameters jUnitCore } } } - + private static void closeIfConfigurable( Computer computer ) throws TestSetFailedException {