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 2014/10/16 16:13:58 UTC

git commit: [SUREFIRE-1095] NPE in JUnit 4.x RunListener + Refactoring

Repository: maven-surefire
Updated Branches:
  refs/heads/master 254e7390a -> 4df651657


[SUREFIRE-1095] NPE in JUnit 4.x RunListener + Refactoring


Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo
Commit: http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/4df65165
Tree: http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/4df65165
Diff: http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/4df65165

Branch: refs/heads/master
Commit: 4df65165717126c88569e1fa62b0ea30559cbfa3
Parents: 254e739
Author: tibordigana <ti...@lycos.com>
Authored: Thu Oct 16 15:20:42 2014 +0200
Committer: tibordigana <ti...@lycos.com>
Committed: Thu Oct 16 15:20:42 2014 +0200

----------------------------------------------------------------------
 .../util/SurefireReflectionException.java       |   2 +-
 .../its/jiras/Surefire1095NpeInRunListener.java |  93 ++++++++++++++
 .../surefire-1095-npe-in-runlistener/pom.xml    |  74 +++++++++++
 .../test/java/jiras/surefire1095/Listener.java  |  38 ++++++
 .../test/java/jiras/surefire1095/SomeTest.java  |  31 +++++
 .../common/junit4/JUnit4ProviderUtil.java       |  11 +-
 .../surefire/common/junit4/JUnit4Reflector.java |  23 ++++
 .../maven/surefire/junit4/JUnit4Provider.java   | 122 ++++++++-----------
 .../surefire/junitcore/JUnitCoreProvider.java   |  43 +++----
 .../junitcore/NonConcurrentRunListener.java     |  12 +-
 10 files changed, 338 insertions(+), 111 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-api/src/main/java/org/apache/maven/surefire/util/SurefireReflectionException.java
----------------------------------------------------------------------
diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/util/SurefireReflectionException.java b/surefire-api/src/main/java/org/apache/maven/surefire/util/SurefireReflectionException.java
index 6c75e8f..ca5d2be 100644
--- a/surefire-api/src/main/java/org/apache/maven/surefire/util/SurefireReflectionException.java
+++ b/surefire-api/src/main/java/org/apache/maven/surefire/util/SurefireReflectionException.java
@@ -20,7 +20,7 @@ package org.apache.maven.surefire.util;
  */
 
 /**
- * Exception indicating that surefire hard problems with reflection. This may be due
+ * Exception indicating that surefire had problems with reflection. This may be due
  * to programming errors, incorrect configuration or incorrect dependencies, but is
  * generally not recoverable and not relevant to handle.
  *

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1095NpeInRunListener.java
----------------------------------------------------------------------
diff --git a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1095NpeInRunListener.java b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1095NpeInRunListener.java
new file mode 100644
index 0000000..71a6ced
--- /dev/null
+++ b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1095NpeInRunListener.java
@@ -0,0 +1,93 @@
+package org.apache.maven.surefire.its.jiras;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase;
+import org.apache.maven.surefire.its.fixture.SurefireLauncher;
+import org.junit.Test;
+
+/**
+ *
+ * In the surefire plugin, it is possible to specify one or more RunListener when running tests with JUnit.
+ * However, it does not look like the listener is properly called by the plugin. In particular, there is a problem
+ * with the method:
+ * <pre>
+ * public void testRunStarted(Description description)
+ * </pre>
+ * it's javadoc at
+ * <a href="http://junit.sourceforge.net/javadoc/org/junit/runner/notification/RunListener.html#testRunStarted%28org.junit.runner.Description%29"/>
+ * states:
+ * "Parameters:
+ * description - describes the tests to be run "
+ * however, in all maven projects I tried ("mvn test"), the surefire plugin seems like passing a null reference instead
+ * of a Description instance that "describes the tests to be run "
+ * Note: other methods in the RunListener I tested seems fine (i.e., they get a valid Description object as input)
+ *
+ * @author <a href="mailto:tibor.digana@gmail.com">Tibor Digana (tibor17)</a>
+ * @see {@linkplain https://jira.codehaus.org/browse/SUREFIRE-1095}
+ * @since 2.18
+ */
+public final class Surefire1095NpeInRunListener
+    extends SurefireJUnit4IntegrationTestCase
+{
+
+    /**
+     * Method Request.classes( String, Class[] ); exists in JUnit 4.0 - 4.4
+     * @see JUnit4Reflector
+     */
+    @Test
+    public void testRunStartedWithJUnit40()
+    {
+        unpack().setJUnitVersion( "4.0" )
+            .executeTest()
+            .verifyErrorFree( 1 )
+            .verifyTextInLog( "Running JUnit 4.0" )
+            .verifyTextInLog( "testRunStarted [jiras.surefire1095.SomeTest]" );
+    }
+
+    /**
+     * Method Request.classes( Class[] ); Since of JUnit 4.5
+     * @see JUnit4Reflector
+     */
+    @Test
+    public void testRunStartedWithJUnit45()
+    {
+        unpack().setJUnitVersion( "4.5" )
+            .executeTest()
+            .verifyErrorFree( 1 )
+            .verifyTextInLog( "Running JUnit 4.5" )
+            .verifyTextInLog( "testRunStarted [jiras.surefire1095.SomeTest]" );
+    }
+
+    @Test
+    public void testRunStartedWithJUnit47()
+    {
+        unpack().setJUnitVersion( "4.7" )
+            .executeTest()
+            .verifyErrorFree( 1 )
+            .verifyTextInLog( "Running JUnit 4.7" )
+            .verifyTextInLog( "testRunStarted [jiras.surefire1095.SomeTest]" );
+    }
+
+    private SurefireLauncher unpack()
+    {
+        return unpack( "surefire-1095-npe-in-runlistener" );
+    }
+}

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/pom.xml
----------------------------------------------------------------------
diff --git a/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/pom.xml b/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/pom.xml
new file mode 100644
index 0000000..4bcd000
--- /dev/null
+++ b/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/pom.xml
@@ -0,0 +1,74 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one
+  ~ or more contributor license agreements.  See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership.  The ASF licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License.  You may obtain a copy of the License at
+  ~
+  ~     http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied.  See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+  -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.apache.maven.surefire</groupId>
+    <artifactId>it-parent</artifactId>
+    <version>1.0</version>
+    <relativePath>../pom.xml</relativePath>
+  </parent>
+  <groupId>org.apache.maven.plugins.surefire</groupId>
+  <artifactId>jiras-surefire-1095</artifactId>
+  <version>1.0</version>
+  <url>http://maven.apache.org</url>
+  <contributors>
+    <contributor>
+      <name>Tibor Digana (tibor17)</name>
+      <email>tibor.digana@gmail.com</email>
+      <timezone>+1</timezone>
+    </contributor>
+  </contributors>
+  <dependencies>
+    <dependency>
+      <groupId>junit</groupId>
+      <artifactId>junit</artifactId>
+      <version>${junit.version}</version>
+      <scope>test</scope>
+    </dependency>
+  </dependencies>
+  <build>
+    <plugins>
+      <plugin>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <version>2.5.1</version>
+        <configuration>
+          <source>1.5</source>
+          <target>1.5</target>
+        </configuration>
+      </plugin>
+      <plugin>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <configuration>
+          <includes>
+            <include>**/SomeTest.java</include>
+          </includes>
+          <properties>
+            <property>
+              <name>listener</name>
+              <value>jiras.surefire1095.Listener</value>
+            </property>
+          </properties>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+</project>

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/Listener.java
----------------------------------------------------------------------
diff --git a/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/Listener.java b/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/Listener.java
new file mode 100644
index 0000000..f16c04a
--- /dev/null
+++ b/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/Listener.java
@@ -0,0 +1,38 @@
+package jiras.surefire1095;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.junit.runner.Description;
+import org.junit.runner.notification.RunListener;
+
+public class Listener
+    extends RunListener
+{
+    @Override
+    public void testRunStarted( Description description )
+        throws Exception
+    {
+        String described = description.getDisplayName();
+        System.out.println( "testRunStarted " +
+                                ( described == null || described.equals( "null" )
+                                    ? description.getChildren()
+                                    : description ) );
+    }
+}

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/SomeTest.java
----------------------------------------------------------------------
diff --git a/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/SomeTest.java b/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/SomeTest.java
new file mode 100644
index 0000000..923917a
--- /dev/null
+++ b/surefire-integration-tests/src/test/resources/surefire-1095-npe-in-runlistener/src/test/java/jiras/surefire1095/SomeTest.java
@@ -0,0 +1,31 @@
+package jiras.surefire1095;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import junit.runner.Version;
+import org.junit.Test;
+
+public class SomeTest {
+    @Test
+    public void test()
+    {
+        System.out.println( "Running JUnit " + Version.id() );
+    }
+}

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4ProviderUtil.java
----------------------------------------------------------------------
diff --git a/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4ProviderUtil.java b/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4ProviderUtil.java
index 52ce708..1e3852a 100644
--- a/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4ProviderUtil.java
+++ b/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4ProviderUtil.java
@@ -22,13 +22,14 @@ package org.apache.maven.surefire.common.junit4;
 import org.apache.maven.surefire.util.TestsToRun;
 import org.apache.maven.surefire.util.internal.StringUtils;
 
-import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.junit.runner.Description;
 import org.junit.runner.notification.Failure;
 
 import static org.apache.maven.surefire.common.junit4.JUnit4RunListener.isFailureInsideJUnitItself;
@@ -106,4 +107,12 @@ public class JUnit4ProviderUtil
         return failingMethods;
     }
 
+    public static Description createSuiteDescription( Collection<Class<?>> classes )
+    {
+        JUnit4Reflector reflector = new JUnit4Reflector();
+        return reflector.createRequest( classes.toArray( new Class[classes.size()] ) )
+                .getRunner()
+                .getDescription();
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4Reflector.java
----------------------------------------------------------------------
diff --git a/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4Reflector.java b/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4Reflector.java
index fdcca6a..0a69703 100644
--- a/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4Reflector.java
+++ b/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/JUnit4Reflector.java
@@ -19,11 +19,14 @@ package org.apache.maven.surefire.common.junit4;
  * under the License.
  */
 
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import org.apache.maven.surefire.util.ReflectionUtils;
 
+import org.apache.maven.surefire.util.SurefireReflectionException;
 import org.junit.Ignore;
 import org.junit.runner.Description;
+import org.junit.runner.Request;
 
 public final class JUnit4Reflector
 {
@@ -49,4 +52,24 @@ public final class JUnit4Reflector
         return ignore != null ? ignore.value() : null;
     }
 
+    public Request createRequest( Class<?>... classes )
+    {
+        try {
+            return (Request) Request.class.getDeclaredMethod( "classes", Class[].class )// Since of JUnit 4.5
+                .invoke( null, new Object[]{ classes } );
+        }
+        catch ( NoSuchMethodException e )
+        {
+            return Request.classes( null, classes );// Since of JUnit 4.0
+        }
+        catch ( InvocationTargetException e )
+        {
+            throw new SurefireReflectionException( e.getCause() );
+        }
+        catch ( IllegalAccessException e )
+        {
+            // probably JUnit 5.x
+            throw new SurefireReflectionException( e );
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
----------------------------------------------------------------------
diff --git a/surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java b/surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
index 03012bd..a9b2799 100644
--- a/surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
+++ b/surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
@@ -41,17 +41,16 @@ import org.apache.maven.surefire.util.RunOrderCalculator;
 import org.apache.maven.surefire.util.ScanResult;
 import org.apache.maven.surefire.util.TestsToRun;
 import org.apache.maven.surefire.util.internal.StringUtils;
+import org.junit.runner.Description;
 import org.junit.runner.Request;
 import org.junit.runner.Result;
-import org.junit.runner.Runner;
-import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunNotifier;
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
 /**
@@ -68,8 +67,6 @@ public class JUnit4Provider
 
     private final String requestedTestMethod;
 
-    private TestsToRun testsToRun;
-
     private final ProviderParameters providerParameters;
 
     private final RunOrderCalculator runOrderCalculator;
@@ -78,13 +75,14 @@ public class JUnit4Provider
 
     private final int rerunFailingTestsCount;
 
+    private TestsToRun testsToRun;
 
     public JUnit4Provider( ProviderParameters booterParameters )
     {
-        this.providerParameters = booterParameters;
-        this.testClassLoader = booterParameters.getTestClassLoader();
-        this.scanResult = booterParameters.getScanResult();
-        this.runOrderCalculator = booterParameters.getRunOrderCalculator();
+        providerParameters = booterParameters;
+        testClassLoader = booterParameters.getTestClassLoader();
+        scanResult = booterParameters.getScanResult();
+        runOrderCalculator = booterParameters.getRunOrderCalculator();
         customRunListeners = JUnit4RunListenerFactory.
             createCustomListeners( booterParameters.getProviderProperties().getProperty( "listener" ) );
         jUnit4TestChecker = new JUnit4TestChecker( testClassLoader );
@@ -122,50 +120,40 @@ public class JUnit4Provider
         JUnit4RunListener jUnit4TestSetReporter = new JUnit4RunListener( reporter );
 
         Result result = new Result();
-        RunNotifier runNotifer = getRunNotifer( jUnit4TestSetReporter, result, customRunListeners );
+        RunNotifier runNotifier = getRunNotifier( jUnit4TestSetReporter, result, customRunListeners );
 
-        runNotifer.fireTestRunStarted( null );
+        runNotifier.fireTestRunStarted( createTestsDescription() );
 
         for ( Class aTestsToRun : testsToRun )
         {
-            executeTestSet( aTestsToRun, reporter, runNotifer, null );
+            executeTestSet( aTestsToRun, reporter, runNotifier );
         }
 
-        runNotifer.fireTestRunFinished( result );
+        runNotifier.fireTestRunFinished( result );
 
         JUnit4RunListener.rethrowAnyTestMechanismFailures( result );
 
-        closeRunNotifer( jUnit4TestSetReporter, customRunListeners );
+        closeRunNotifier( jUnit4TestSetReporter, customRunListeners );
         return reporterFactory.close();
     }
 
-    private void executeTestSet( Class<?> clazz, RunListener reporter, RunNotifier listeners,
-                                 String[] failingTestMethods )
-        throws ReporterException, TestSetFailedException
+    private void executeTestSet( Class<?> clazz, RunListener reporter, RunNotifier listeners )
     {
-        final ReportEntry report = new SimpleReportEntry( this.getClass().getName(), clazz.getName() );
-
+        final ReportEntry report = new SimpleReportEntry( getClass().getName(), clazz.getName() );
         reporter.testSetStarting( report );
-
-
-
         try
         {
-            if ( !StringUtils.isBlank( this.requestedTestMethod ) )
+            if ( !StringUtils.isBlank( requestedTestMethod ) )
             {
-                String actualTestMethod = getMethod( clazz, this.requestedTestMethod );//add by rainLee
+                String actualTestMethod = getMethod( clazz, requestedTestMethod );//add by rainLee
                 String[] testMethods = StringUtils.split( actualTestMethod, "+" );
                 executeWithRerun( clazz, listeners, testMethods );
             }
             else
             {//the original way
-                executeWithRerun( clazz, listeners, failingTestMethods );
+                executeWithRerun( clazz, listeners, null );
             }
         }
-        catch ( TestSetFailedException e )
-        {
-            throw e;
-        }
         catch ( Throwable e )
         {
             reporter.testError( SimpleReportEntry.withException( report.getSourceName(), report.getName(),
@@ -179,7 +167,6 @@ public class JUnit4Provider
     }
 
     private void executeWithRerun( Class<?> clazz, RunNotifier listeners, String[] testMethods )
-        throws TestSetFailedException
     {
         JUnitTestFailureListener failureListener = new JUnitTestFailureListener();
         listeners.addListener( failureListener );
@@ -187,27 +174,20 @@ public class JUnit4Provider
         execute( clazz, listeners, testMethods );
 
         // Rerun failing tests if rerunFailingTestsCount is larger than 0
-        int rerunCount = this.rerunFailingTestsCount;
-        if ( rerunCount > 0 )
+        if ( rerunFailingTestsCount > 0 )
         {
-            for ( int i = 0; i < rerunCount; i++ )
+            for ( int i = 0; i < rerunFailingTestsCount && !failureListener.getAllFailures().isEmpty(); i++ )
             {
-                List<Failure> failures = failureListener.getAllFailures();
-                if ( failures.size() == 0 )
-                {
-                    break;
-                }
-
                 Set<String> methodsSet = JUnit4ProviderUtil.generateFailingTests( failureListener.getAllFailures() );
-                String[] methods = methodsSet.toArray(new String[methodsSet.size()]);
+                String[] methods = methodsSet.toArray( new String[ methodsSet.size() ] );
                 failureListener.reset();
                 execute( clazz, listeners, methods );
             }
         }
     }
 
-    private RunNotifier getRunNotifer( org.junit.runner.notification.RunListener main, Result result,
-                                       List<org.junit.runner.notification.RunListener> others )
+    private RunNotifier getRunNotifier( org.junit.runner.notification.RunListener main, Result result,
+                                        List<org.junit.runner.notification.RunListener> others )
     {
         RunNotifier fNotifier = new RunNotifier();
         fNotifier.addListener( main );
@@ -219,10 +199,10 @@ public class JUnit4Provider
         return fNotifier;
     }
 
-    // I am not entierly sure as to why we do this explicit freeing, it's one of those
+    // I am not entirely sure as to why we do this explicit freeing, it's one of those
     // pieces of code that just seem to linger on in here ;)
-    private void closeRunNotifer( org.junit.runner.notification.RunListener main,
-                                  List<org.junit.runner.notification.RunListener> others )
+    private void closeRunNotifier( org.junit.runner.notification.RunListener main,
+                                   List<org.junit.runner.notification.RunListener> others )
     {
         RunNotifier fNotifier = new RunNotifier();
         fNotifier.removeListener( main );
@@ -248,7 +228,7 @@ public class JUnit4Provider
     private void upgradeCheck()
         throws TestSetFailedException
     {
-        if ( isJunit4UpgradeCheck() )
+        if ( isJUnit4UpgradeCheck() )
         {
             List<Class> classesSkippedByValidation =
                 scanResult.getClassesSkippedByValidation( jUnit4TestChecker, testClassLoader );
@@ -268,37 +248,41 @@ public class JUnit4Provider
         }
     }
 
-    private boolean isJunit4UpgradeCheck()
+    private Description createTestsDescription()
     {
-        final String property = System.getProperty( "surefire.junit4.upgradecheck" );
-        return property != null;
+        Collection<Class<?>> classes = new ArrayList<Class<?>>();
+        for ( Class<?> clazz : testsToRun )
+        {
+            classes.add( clazz );
+        }
+        return JUnit4ProviderUtil.createSuiteDescription( classes );
     }
 
+    private static boolean isJUnit4UpgradeCheck()
+    {
+        return System.getProperty( "surefire.junit4.upgradecheck" ) != null;
+    }
 
     private static void execute( Class<?> testClass, RunNotifier fNotifier, String[] testMethods )
-        throws TestSetFailedException
     {
-        if ( null != testMethods )
+        if ( testMethods != null )
         {
-            Method[] methods = testClass.getMethods();
-            for ( Method method : methods )
+            for ( final Method method : testClass.getMethods() )
             {
-                for ( String testMethod : testMethods )
+                for ( final String testMethod : testMethods )
                 {
                     if ( SelectorUtils.match( testMethod, method.getName() ) )
                     {
-                        Runner junitTestRunner = Request.method( testClass, method.getName() ).getRunner();
-                        junitTestRunner.run( fNotifier );
+                        Request.method( testClass, method.getName() ).getRunner().run( fNotifier );
                     }
 
                 }
             }
-            return;
         }
-
-        Runner junitTestRunner = Request.aClass( testClass ).getRunner();
-
-        junitTestRunner.run( fNotifier );
+        else
+        {
+            Request.aClass( testClass ).getRunner().run( fNotifier );
+        }
     }
 
     /**
@@ -312,23 +296,19 @@ public class JUnit4Provider
      */
     private static String getMethod( Class testClass, String testMethodStr )
     {
-        String className = testClass.getName();
+        final String className = testClass.getName();
 
         if ( !testMethodStr.contains( "#" ) && !testMethodStr.contains( "," ) )
         {//the original way
             return testMethodStr;
         }
         testMethodStr += ",";//for the bellow  split code
-        int beginIndex = testMethodStr.indexOf( className );
-        int endIndex = testMethodStr.indexOf( ",", beginIndex );
-        String classMethodStr =
+        final int beginIndex = testMethodStr.indexOf( className );
+        final int endIndex = testMethodStr.indexOf( ",", beginIndex );
+        final String classMethodStr =
             testMethodStr.substring( beginIndex, endIndex );//String like "StopWatchTest#testLang315"
 
-        int index = classMethodStr.indexOf( '#' );
-        if ( index >= 0 )
-        {
-            return classMethodStr.substring( index + 1, classMethodStr.length() );
-        }
-        return null;
+        final int index = classMethodStr.indexOf( '#' );
+        return index >= 0 ? classMethodStr.substring( index + 1, classMethodStr.length() ) : null;
     }
 }

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/JUnitCoreProvider.java
----------------------------------------------------------------------
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 8cbf0cd..a145b44 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
@@ -19,8 +19,6 @@ package org.apache.maven.surefire.junitcore;
  * under the License.
  */
 
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -48,11 +46,7 @@ import org.apache.maven.surefire.util.ScanResult;
 import org.apache.maven.surefire.util.ScannerFilter;
 import org.apache.maven.surefire.util.TestsToRun;
 import org.apache.maven.surefire.util.internal.StringUtils;
-import org.junit.internal.runners.statements.Fail;
 import org.junit.runner.manipulation.Filter;
-import org.junit.runner.notification.Failure;
-
-import static org.apache.maven.surefire.common.junit4.JUnit4RunListener.isFailureInsideJUnitItself;
 
 /**
  * @author Kristian Rosenvold
@@ -71,6 +65,10 @@ public class JUnitCoreProvider
 
     private final ProviderParameters providerParameters;
 
+    private final ScanResult scanResult;
+
+    private final int rerunFailingTestsCount;
+
     private TestsToRun testsToRun;
 
     private JUnit48Reflector jUnit48Reflector;
@@ -79,20 +77,16 @@ public class JUnitCoreProvider
 
     private String requestedTestMethod;
 
-    private final ScanResult scanResult;
-
-    private final int rerunFailingTestsCount;
-
     public JUnitCoreProvider( ProviderParameters providerParameters )
     {
         this.providerParameters = providerParameters;
-        this.testClassLoader = providerParameters.getTestClassLoader();
-        this.scanResult = providerParameters.getScanResult();
-        this.runOrderCalculator = providerParameters.getRunOrderCalculator();
-        this.jUnitCoreParameters = new JUnitCoreParameters( providerParameters.getProviderProperties() );
-        this.scannerFilter = new JUnit48TestChecker( testClassLoader );
-        this.requestedTestMethod = providerParameters.getTestRequest().getRequestedTestMethod();
-        this.rerunFailingTestsCount = providerParameters.getTestRequest().getRerunFailingTestsCount();
+        testClassLoader = providerParameters.getTestClassLoader();
+        scanResult = providerParameters.getScanResult();
+        runOrderCalculator = providerParameters.getRunOrderCalculator();
+        jUnitCoreParameters = new JUnitCoreParameters( providerParameters.getProviderProperties() );
+        scannerFilter = new JUnit48TestChecker( testClassLoader );
+        requestedTestMethod = providerParameters.getTestRequest().getRequestedTestMethod();
+        rerunFailingTestsCount = providerParameters.getTestRequest().getRerunFailingTestsCount();
 
         customRunListeners =
             JUnit4RunListenerFactory.createCustomListeners( providerParameters.getProviderProperties().getProperty( "listener" ) );
@@ -141,8 +135,7 @@ public class JUnitCoreProvider
             }
         }
 
-        org.junit.runner.notification.RunListener jUnit4RunListener = getRunListener( reporterFactory, consoleLogger );
-        customRunListeners.add( 0, jUnit4RunListener );
+        customRunListeners.add( 0, getRunListener( reporterFactory, consoleLogger ) );
 
         // Add test failure listener
         JUnitTestFailureListener testFailureListener = new JUnitTestFailureListener();
@@ -151,18 +144,12 @@ public class JUnitCoreProvider
         JUnitCoreWrapper.execute( testsToRun, jUnitCoreParameters, customRunListeners, filter );
 
         // Rerun failing tests if rerunFailingTestsCount is larger than 0
-        int rerunCount = this.rerunFailingTestsCount;
-        if ( rerunCount > 0 )
+        if ( rerunFailingTestsCount > 0 )
         {
-            for ( int i = 0; i < rerunCount; i++ )
+            for ( int i = 0; i < rerunFailingTestsCount && !testFailureListener.getAllFailures().isEmpty(); i++ )
             {
-                List<Failure> failures = testFailureListener.getAllFailures();
-                if ( failures.size() == 0 )
-                {
-                    break;
-                }
                 Map<Class<?>, Set<String>> failingTests =
-                    JUnit4ProviderUtil.generateFailingTests( failures, testsToRun );
+                    JUnit4ProviderUtil.generateFailingTests( testFailureListener.getAllFailures(), testsToRun );
                 testFailureListener.reset();
                 final FilterFactory filterFactory = new FilterFactory( testClassLoader );
                 Filter failingMethodsFilter = filterFactory.createFailingMethodFilter( failingTests );

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/4df65165/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/NonConcurrentRunListener.java
----------------------------------------------------------------------
diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/NonConcurrentRunListener.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/NonConcurrentRunListener.java
index db2ff05..458b484 100644
--- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/NonConcurrentRunListener.java
+++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/NonConcurrentRunListener.java
@@ -56,20 +56,12 @@ public class NonConcurrentRunListener
 
     protected SimpleReportEntry createReportEntry( Description description )
     {
-        return new SimpleReportEntry( description.getClassName(), description.getDisplayName()/*
-                                                                                               * , (int) (
-                                                                                               * System.currentTimeMillis
-                                                                                               * () - startTime )
-                                                                                               */);
+        return new SimpleReportEntry( description.getClassName(), description.getDisplayName() );
     }
 
     protected SimpleReportEntry createReportEntryForTestSet( Description description )
     {
-        return new SimpleReportEntry( description.getClassName(), description.getClassName() /*
-                                                                                              * , (int) (
-                                                                                              * System.currentTimeMillis
-                                                                                              * () - startTime )
-                                                                                              */);
+        return new SimpleReportEntry( description.getClassName(), description.getClassName() );
     }
 
     @Override