[SUREFIRE] refactoring + properly closing TestProvidingInputStream

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

Branch: refs/heads/master
Commit: 2c04ad8b88c5a82f5e80243c3d88604ef4ab95cf
Parents: 4d7be01
Author: Tibor17 <[email protected]>
Authored: Wed Jul 15 23:12:04 2015 +0200
Committer: Tibor17 <[email protected]>
Committed: Thu Jul 23 23:28:14 2015 +0200

----------------------------------------------------------------------
 .../surefire/booterclient/ForkStarter.java      | 119 ++++++++-----------
 .../surefire/report/DefaultReporterFactory.java |   9 +-
 2 files changed, 52 insertions(+), 76 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/2c04ad8b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
----------------------------------------------------------------------
diff --git 
a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
 
b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
index 64bce8b..804a08e 100644
--- 
a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
+++ 
b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
@@ -135,7 +135,7 @@ public class ForkStarter
 
     private final DefaultReporterFactory defaultReporterFactory;
 
-    private final Collection<DefaultReporterFactory> 
defaultReporterFactoryList;
+    private final Collection<DefaultReporterFactory> defaultReporterFactories;
 
     private static volatile int systemPropertiesFileCounter = 0;
 
@@ -151,7 +151,7 @@ public class ForkStarter
         this.log = log;
         defaultReporterFactory = new DefaultReporterFactory( 
startupReportConfiguration );
         defaultReporterFactory.runStarting();
-        defaultReporterFactoryList = new 
ConcurrentLinkedQueue<DefaultReporterFactory>();
+        defaultReporterFactories = new 
ConcurrentLinkedQueue<DefaultReporterFactory>();
     }
 
     public RunResult run( SurefireProperties effectiveSystemProperties, 
DefaultScanResult scanResult )
@@ -167,7 +167,7 @@ public class ForkStarter
         }
         finally
         {
-            defaultReporterFactory.mergeFromOtherFactories( 
defaultReporterFactoryList );
+            defaultReporterFactory.mergeFromOtherFactories( 
defaultReporterFactories );
             defaultReporterFactory.close();
         }
     }
@@ -176,7 +176,7 @@ public class ForkStarter
             throws SurefireBooterForkException
     {
         DefaultReporterFactory forkedReporterFactory = new 
DefaultReporterFactory( startupReportConfiguration );
-        defaultReporterFactoryList.add( forkedReporterFactory );
+        defaultReporterFactories.add( forkedReporterFactory );
         final ForkClient forkClient =
                 new ForkClient( forkedReporterFactory, 
startupReportConfiguration.getTestVmSystemProperties() );
         return fork( null, new PropertiesWrapper( providerProperties ), 
forkClient, effectiveSystemProperties, null );
@@ -206,15 +206,13 @@ public class ForkStarter
     private RunResult runSuitesForkOnceMultiple( final SurefireProperties 
effectiveSystemProperties, int forkCount )
         throws SurefireBooterForkException
     {
-
         ArrayList<Future<RunResult>> results = new 
ArrayList<Future<RunResult>>( forkCount );
         ThreadPoolExecutor executorService = new ThreadPoolExecutor( 
forkCount, forkCount, 60, TimeUnit.SECONDS,
                                                                   new 
ArrayBlockingQueue<Runnable>( forkCount ) );
         executorService.setThreadFactory( threadFactory );
+        Collection<TestProvidingInputStream> testStreams = new 
ArrayList<TestProvidingInputStream>();
         try
         {
-            // Ask to the executorService to run all tasks
-            RunResult globalResult = new RunResult( 0, 0, 0, 0 );
             final Queue<String> messageQueue = new 
ConcurrentLinkedQueue<String>();
             for ( Class<?> clazz : getSuitesIterator() )
             {
@@ -223,17 +221,16 @@ public class ForkStarter
 
             for ( int forkNum = 0, total = messageQueue.size(); forkNum < 
forkCount && forkNum < total; forkNum++ )
             {
+                final TestProvidingInputStream testProvidingInputStream = new 
TestProvidingInputStream( messageQueue );
+                testStreams.add( testProvidingInputStream );
                 Callable<RunResult> pf = new Callable<RunResult>()
                 {
                     public RunResult call()
                         throws Exception
                     {
-                        TestProvidingInputStream testProvidingInputStream =
-                                new TestProvidingInputStream( messageQueue );
-
                         DefaultReporterFactory forkedReporterFactory =
                             new DefaultReporterFactory( 
startupReportConfiguration );
-                        defaultReporterFactoryList.add( forkedReporterFactory 
);
+                        defaultReporterFactories.add( forkedReporterFactory );
                         ForkClient forkClient = new ForkClient( 
forkedReporterFactory,
                                                                 
startupReportConfiguration.getTestVmSystemProperties(),
                                                                 
testProvidingInputStream );
@@ -242,47 +239,19 @@ public class ForkStarter
                                      forkClient, effectiveSystemProperties, 
testProvidingInputStream );
                     }
                 };
-
                 results.add( executorService.submit( pf ) );
             }
-
-            for ( Future<RunResult> result : results )
-            {
-                try
-                {
-                    RunResult cur = result.get();
-                    if ( cur != null )
-                    {
-                        globalResult = globalResult.aggregate( cur );
-                    }
-                    else
-                    {
-                        throw new SurefireBooterForkException( "No results for 
" + result.toString() );
-                    }
-                }
-                catch ( InterruptedException e )
-                {
-                    executorService.shutdownNow();
-                    Thread.currentThread().interrupt();
-                    throw new SurefireBooterForkException( "Interrupted", e );
-                }
-                catch ( ExecutionException e )
-                {
-                    throw new SurefireBooterForkException( 
"ExecutionException", e );
-                }
-            }
-            return globalResult;
-
+            return awaitResultsDone( results, executorService );
         }
         finally
         {
+            closeTestStreams( testStreams );
             closeExecutor( executorService );
         }
-
     }
 
     @SuppressWarnings( "checkstyle:magicnumber" )
-    private RunResult runSuitesForkPerTestSet( final SurefireProperties 
effectiveSystemProperties, final int forkCount )
+    private RunResult runSuitesForkPerTestSet( final SurefireProperties 
effectiveSystemProperties, int forkCount )
         throws SurefireBooterForkException
     {
         ArrayList<Future<RunResult>> results = new 
ArrayList<Future<RunResult>>( 500 );
@@ -291,9 +260,6 @@ public class ForkStarter
         executorService.setThreadFactory( threadFactory );
         try
         {
-            // Ask to the executorService to run all tasks
-            RunResult globalResult = new RunResult( 0, 0, 0, 0 );
-
             for ( final Object testSet : getSuitesIterator() )
             {
                 Callable<RunResult> pf = new Callable<RunResult>()
@@ -303,7 +269,7 @@ public class ForkStarter
                     {
                         DefaultReporterFactory forkedReporterFactory =
                             new DefaultReporterFactory( 
startupReportConfiguration );
-                        defaultReporterFactoryList.add( forkedReporterFactory 
);
+                        defaultReporterFactories.add( forkedReporterFactory );
                         ForkClient forkClient = new ForkClient( 
forkedReporterFactory,
                                                         
startupReportConfiguration.getTestVmSystemProperties() );
                         return fork( testSet, new PropertiesWrapper( 
providerConfiguration.getProviderProperties() ),
@@ -311,42 +277,53 @@ public class ForkStarter
                     }
                 };
                 results.add( executorService.submit( pf ) );
-
             }
+            return awaitResultsDone( results, executorService );
+        }
+        finally
+        {
+            closeExecutor( executorService );
+        }
+    }
 
-            for ( Future<RunResult> result : results )
+    private static RunResult awaitResultsDone( Collection<Future<RunResult>> 
results, ExecutorService executorService )
+        throws SurefireBooterForkException
+    {
+        RunResult globalResult = new RunResult( 0, 0, 0, 0 );
+        for ( Future<RunResult> result : results )
+        {
+            try
             {
-                try
-                {
-                    RunResult cur = result.get();
-                    if ( cur != null )
-                    {
-                        globalResult = globalResult.aggregate( cur );
-                    }
-                    else
-                    {
-                        throw new SurefireBooterForkException( "No results for 
" + result.toString() );
-                    }
-                }
-                catch ( InterruptedException e )
+                RunResult cur = result.get();
+                if ( cur != null )
                 {
-                    executorService.shutdownNow();
-                    Thread.currentThread().interrupt();
-                    throw new SurefireBooterForkException( "Interrupted", e );
+                    globalResult = globalResult.aggregate( cur );
                 }
-                catch ( ExecutionException e )
+                else
                 {
-                    throw new SurefireBooterForkException( 
"ExecutionException", e );
+                    throw new SurefireBooterForkException( "No results for " + 
result.toString() );
                 }
             }
-            return globalResult;
-
+            catch ( InterruptedException e )
+            {
+                executorService.shutdownNow();
+                Thread.currentThread().interrupt();
+                throw new SurefireBooterForkException( "Interrupted", e );
+            }
+            catch ( ExecutionException e )
+            {
+                throw new SurefireBooterForkException( "ExecutionException", e 
);
+            }
         }
-        finally
+        return globalResult;
+    }
+
+    private static void closeTestStreams( Iterable<TestProvidingInputStream> 
testStreams )
+    {
+        for ( TestProvidingInputStream testStream: testStreams )
         {
-            closeExecutor( executorService );
+            testStream.close();
         }
-
     }
 
     @SuppressWarnings( "checkstyle:magicnumber" )

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/2c04ad8b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java
----------------------------------------------------------------------
diff --git 
a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java
 
b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java
index aeb3c22..2af0b40 100644
--- 
a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java
+++ 
b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java
@@ -30,11 +30,11 @@ import org.apache.maven.surefire.suite.RunResult;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
+import java.util.concurrent.ConcurrentLinkedQueue;
 
 /**
  * Provides reporting modules on the plugin side.
@@ -53,8 +53,7 @@ public class DefaultReporterFactory
 
     private final StatisticsReporter statisticsReporter;
 
-    private final List<TestSetRunListener> listeners =
-        Collections.synchronizedList( new ArrayList<TestSetRunListener>() );
+    private final Collection<TestSetRunListener> listeners = new 
ConcurrentLinkedQueue<TestSetRunListener>();
 
     // from "<testclass>.<testmethod>" -> statistics about all the runs for 
flaky tests
     private Map<String, List<TestMethodStats>> flakyTests;
@@ -97,11 +96,11 @@ public class DefaultReporterFactory
                                     reportConfiguration.isTrimStackTrace(),
                                     ConsoleReporter.PLAIN.equals( 
reportConfiguration.getReportFormat() ),
                                     reportConfiguration.isBriefOrPlainFormat() 
);
-        listeners.add( testSetRunListener );
+        addListener( testSetRunListener );
         return testSetRunListener;
     }
 
-    public void addListener( TestSetRunListener listener )
+    final void addListener( TestSetRunListener listener )
     {
         listeners.add( listener );
     }

Reply via email to