You are viewing a plain text version of this content. The canonical link for it is here.
Posted to surefire-commits@maven.apache.org by kr...@apache.org on 2010/12/05 22:19:24 UTC

svn commit: r1042451 - in /maven/surefire/trunk: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/ maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ maven-surefire-plugin/src/main/java/org/apache/mav...

Author: krosenvold
Date: Sun Dec  5 21:19:24 2010
New Revision: 1042451

URL: http://svn.apache.org/viewvc?rev=1042451&view=rev
Log:
o Simplified the dual code paths creating the classloaders

While there is still room for improvement in this code, there's less
duplication now.

Also cleaned some other minor stuff

Modified:
    maven/surefire/trunk/maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
    maven/surefire/trunk/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
    maven/surefire/trunk/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
    maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/BooterDeserializer.java
    maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java
    maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SurefireStarter.java
    maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SystemPropertyManager.java

Modified: maven/surefire/trunk/maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java?rev=1042451&r1=1042450&r2=1042451&view=diff
==============================================================================
--- maven/surefire/trunk/maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java (original)
+++ maven/surefire/trunk/maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java Sun Dec  5 21:19:24 2010
@@ -608,7 +608,8 @@ public class IntegrationTestMojo
             ForkConfiguration forkConfiguration = getForkConfiguration( bootClasspathConfiguration );
 
             BooterConfiguration booterConfiguration = createBooterConfiguration( forkConfiguration, provider );
-            ForkStarter booter = new ForkStarter( booterConfiguration, reportsDirectory, forkConfiguration );
+            ForkStarter booter = new ForkStarter( booterConfiguration, reportsDirectory, forkConfiguration,
+                                                  getForkedProcessTimeoutInSeconds() );
 
             getLog().info(
                 StringUtils.capitalizeFirstLetter( getPluginName() ) + " report directory: " + getReportsDirectory() );

Modified: maven/surefire/trunk/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java?rev=1042451&r1=1042450&r2=1042451&view=diff
==============================================================================
--- maven/surefire/trunk/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java (original)
+++ maven/surefire/trunk/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java Sun Dec  5 21:19:24 2010
@@ -33,6 +33,7 @@ import org.apache.maven.surefire.booter.
 import org.apache.maven.surefire.booter.SurefireStarter;
 import org.apache.maven.surefire.booter.SystemPropertyManager;
 import org.apache.maven.surefire.providerapi.SurefireProvider;
+import org.apache.maven.surefire.suite.RunResult;
 import org.codehaus.plexus.util.IOUtil;
 import org.codehaus.plexus.util.cli.CommandLineException;
 import org.codehaus.plexus.util.cli.CommandLineUtils;
@@ -61,33 +62,34 @@ import java.util.Properties;
  */
 public class ForkStarter
 {
-    private int forkedProcessTimeoutInSeconds = 0;
+    private final int forkedProcessTimeoutInSeconds;
 
     private final BooterConfiguration booterConfiguration;
 
-
     private final ForkConfiguration forkConfiguration;
 
-    private File reportsDirectory;
+    private final File reportsDirectory;
 
     public ForkStarter( BooterConfiguration booterConfiguration, File reportsDirectory,
-                        ForkConfiguration forkConfiguration )
+                        ForkConfiguration forkConfiguration, int forkedProcessTimeoutInSeconds )
     {
         this.forkConfiguration = forkConfiguration;
         this.booterConfiguration = booterConfiguration;
         this.reportsDirectory = reportsDirectory;
+        this.forkedProcessTimeoutInSeconds = forkedProcessTimeoutInSeconds;
     }
 
     public int run()
         throws SurefireBooterForkException, SurefireExecutionException
     {
-        int result;
+        final int result;
 
         final String requestedForkMode = forkConfiguration.getForkMode();
         if ( ForkConfiguration.FORK_NEVER.equals( requestedForkMode ) )
         {
             SurefireStarter testVmBooter = new SurefireStarter( booterConfiguration );
-            result = testVmBooter.runSuitesInProcess();
+            RunResult runResult = testVmBooter.runSuitesInProcess();
+            result = testVmBooter.processRunCount( runResult );
         }
         else if ( ForkConfiguration.FORK_ONCE.equals( requestedForkMode ) )
         {
@@ -281,9 +283,4 @@ public class ForkStarter
 
         return new ForkingStreamConsumer( outputConsumer );
     }
-
-    public void setForkedProcessTimeoutInSeconds( int forkedProcessTimeoutInSeconds )
-    {
-        this.forkedProcessTimeoutInSeconds = forkedProcessTimeoutInSeconds;
-    }
 }

Modified: maven/surefire/trunk/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java?rev=1042451&r1=1042450&r2=1042451&view=diff
==============================================================================
--- maven/surefire/trunk/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java (original)
+++ maven/surefire/trunk/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java Sun Dec  5 21:19:24 2010
@@ -571,15 +571,15 @@ public class SurefirePlugin
         if ( verifyParameters() )
         {
             final Classpath bootClasspathConfiguration = new Classpath();
-            ForkConfiguration forkConfiguration = getForkConfiguration(bootClasspathConfiguration);
+            ForkConfiguration forkConfiguration = getForkConfiguration( bootClasspathConfiguration );
 
             BooterConfiguration booterConfiguration = createBooterConfiguration( forkConfiguration, provider );
 
             getLog().info(
                 StringUtils.capitalizeFirstLetter( getPluginName() ) + " report directory: " + getReportsDirectory() );
 
-            ForkStarter forkStarter = new ForkStarter( booterConfiguration, reportsDirectory, forkConfiguration );
-            forkStarter.setForkedProcessTimeoutInSeconds( getForkedProcessTimeoutInSeconds() );
+            ForkStarter forkStarter = new ForkStarter( booterConfiguration, reportsDirectory, forkConfiguration,
+                                                       getForkedProcessTimeoutInSeconds() );
 
             int result;
             try

Modified: maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/BooterDeserializer.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/BooterDeserializer.java?rev=1042451&r1=1042450&r2=1042451&view=diff
==============================================================================
--- maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/BooterDeserializer.java (original)
+++ maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/BooterDeserializer.java Sun Dec  5 21:19:24 2010
@@ -25,7 +25,6 @@ import org.apache.maven.surefire.testset
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
@@ -151,7 +150,7 @@ public class BooterDeserializer
             }
             else if ( FORKTESTSET.equals( name ) )
             {
-                testForFork = getTypeDecoded( properties.getProperty( FORKTESTSET ));
+                testForFork = getTypeDecoded( properties.getProperty( FORKTESTSET ) );
             }
             else if ( REQUESTEDTEST.equals( name ) )
             {
@@ -182,8 +181,7 @@ public class BooterDeserializer
                                                            valueOf( failIfNotests ) );
 
         TestArtifactInfo testNg = new TestArtifactInfo( testNgVersion, testNgClassifier );
-        TestRequest testSuiteDefinition =
-            new TestRequest( testSuiteXmlFiles, sourceDirectory, requestedTest );
+        TestRequest testSuiteDefinition = new TestRequest( testSuiteXmlFiles, sourceDirectory, requestedTest );
 
         ClassLoaderConfiguration classLoaderConfiguration =
             new ClassLoaderConfiguration( useSystemClassLoader, useManifestOnlyJar );
@@ -197,13 +195,14 @@ public class BooterDeserializer
             new ReporterConfiguration( reports, reportsDirectory, valueOf( isTrimStackTrace ) );
 
         ProviderConfiguration surefireStarterConfiguration =
-            ProviderConfiguration.inForkedVm( providerConfiguration, classpathConfiguration, classLoaderConfiguration);
+            ProviderConfiguration.inForkedVm( providerConfiguration, classpathConfiguration, classLoaderConfiguration );
         return new BooterConfiguration( surefireStarterConfiguration, reports, dirScannerParams, failIfNotests,
-                                        reporterConfiguration, testNg, testSuiteDefinition,
-                                        properties.getProperties(), testForFork );
+                                        reporterConfiguration, testNg, testSuiteDefinition, properties.getProperties(),
+                                        testForFork );
     }
 
-    private Boolean valueOf(boolean aBoolean){  // jdk1.3 compat
+    private Boolean valueOf( boolean aBoolean )
+    {  // jdk1.3 compat
         return aBoolean ? Boolean.TRUE : Boolean.FALSE;
     }
 
@@ -212,21 +211,6 @@ public class BooterDeserializer
         return name.endsWith( PARAMS_SUFIX ) || name.endsWith( TYPES_SUFIX );
     }
 
-    void writePropertiesFile( File file, String name, Properties properties )
-        throws IOException
-    {
-        FileOutputStream out = new FileOutputStream( file );
-
-        try
-        {
-            properties.store( out, name );
-        }
-        finally
-        {
-            out.close();
-        }
-    }
-
     private static List processStringList( String stringList )
     {
         String sl = stringList;
@@ -267,11 +251,12 @@ public class BooterDeserializer
         return paramObjects;
     }
 
-    private static Object getTypeDecoded(String typeEncoded){
+    private static Object getTypeDecoded( String typeEncoded )
+    {
         int typeSep = typeEncoded.indexOf( "|" );
         String type = typeEncoded.substring( 0, typeSep );
-        String value = typeEncoded.substring(  typeSep + 1);
-        return getParamValue(  value, type );
+        String value = typeEncoded.substring( typeSep + 1 );
+        return getParamValue( value, type );
     }
 
     private static Object getParamValue( String param, String typeName )

Modified: maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java?rev=1042451&r1=1042450&r2=1042451&view=diff
==============================================================================
--- maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java (original)
+++ maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java Sun Dec  5 21:19:24 2010
@@ -19,6 +19,8 @@ package org.apache.maven.surefire.booter
  * under the License.
  */
 
+import org.apache.maven.surefire.suite.RunResult;
+
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.InputStream;
@@ -28,7 +30,7 @@ import java.util.Properties;
  * The part of the booter that is unique to a forked vm.
  * <p/>
  * Deals with deserialization of the booter wire-level protocol
- *
+ * <p/>
  * Todo: Look at relationship between this class and BooterSerializer (BooterDeserializer?)
  *
  * @author Jason van Zyl
@@ -66,16 +68,18 @@ public class ForkedBooter
             Object forkedTestSet = booterConfiguration.getTestForFork();
             Properties p = booterConfiguration.getProviderProperties();
             final int result;
+            final RunResult runResult;
             if ( forkedTestSet != null )
             {
-                result = booter.runSuitesInProcess( forkedTestSet, p );
-                booterDeserializer.writePropertiesFile( surefirePropertiesFile, "surefire", p );
+                runResult = booter.runSuitesInProcess( forkedTestSet );
+                booter.updateResultsProperties( runResult, p );
+                SystemPropertyManager.writePropertiesFile( surefirePropertiesFile, "surefire", p );
             }
             else
             {
-                result = booter.runSuitesInProcess();
+                runResult = booter.runSuitesInProcess();
             }
-
+            result = booter.processRunCount( runResult );
 
             // noinspection CallToSystemExit
             System.exit( result );

Modified: maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SurefireStarter.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SurefireStarter.java?rev=1042451&r1=1042450&r2=1042451&view=diff
==============================================================================
--- maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SurefireStarter.java (original)
+++ maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SurefireStarter.java Sun Dec  5 21:19:24 2010
@@ -48,10 +48,9 @@ public class SurefireStarter
         this.booterConfiguration = booterConfiguration;
     }
 
-    public int runSuitesInProcess( Object testSet, Properties results )
+    public RunResult runSuitesInProcess( Object testSet )
         throws SurefireExecutionException
     {
-
         final ProviderConfiguration starterConfiguration = booterConfiguration.getSurefireStarterConfiguration();
         final ClasspathConfiguration classpathConfiguration = starterConfiguration.getClasspathConfiguration();
 
@@ -60,11 +59,48 @@ public class SurefireStarter
 
         ClassLoader surefireClassLoader = classpathConfiguration.createSurefireClassLoader( testsClassLoader );
 
-        RunResult runResult = invokeProvider( testSet, testsClassLoader, surefireClassLoader );
+        return invokeProvider( testSet, testsClassLoader, surefireClassLoader );
+    }
+
+    public RunResult runSuitesInProcess()
+        throws SurefireExecutionException
+    {
+        // The test classloader must be constructed first to avoid issues with commons-logging until we properly
+        // separate the TestNG classloader
+        ClassLoader testsClassLoader = createInProcessTestClassLoader();
+
+        final ClasspathConfiguration classpathConfiguration =
+            booterConfiguration.getSurefireStarterConfiguration().getClasspathConfiguration();
 
-        updateResultsProperties( runResult, results );
+        ClassLoader surefireClassLoader = classpathConfiguration.createSurefireClassLoader( testsClassLoader );
 
-        return processRunCount( runResult );
+        return invokeProvider( null, testsClassLoader, surefireClassLoader );
+    }
+
+    private ClassLoader createInProcessTestClassLoader()
+        throws SurefireExecutionException
+    {
+        ClassLoader testsClassLoader;
+
+        final ProviderConfiguration starterConfiguration = booterConfiguration.getSurefireStarterConfiguration();
+        final ClasspathConfiguration classpathConfiguration = starterConfiguration.getClasspathConfiguration();
+
+        String testClassPath = classpathConfiguration.getTestClasspath().getClassPathAsString();
+
+        System.setProperty( "surefire.test.class.path", testClassPath );
+        if ( starterConfiguration.isManifestOnlyJarRequestedAndUsable() )
+        {
+            testsClassLoader = getClass().getClassLoader(); // ClassLoader.getSystemClassLoader()
+            // SUREFIRE-459, trick the app under test into thinking its classpath was conventional
+            // (instead of a single manifest-only jar)
+            System.setProperty( "surefire.real.class.path", System.getProperty( "java.class.path" ) );
+            System.setProperty( "java.class.path", testClassPath );
+        }
+        else
+        {
+            testsClassLoader = classpathConfiguration.createTestClassLoader();
+        }
+        return testsClassLoader;
     }
 
     private static final String RESULTS_ERRORS = "errors";
@@ -76,7 +112,7 @@ public class SurefireStarter
     private static final String RESULTS_SKIPPED = "skipped";
 
 
-    public synchronized void updateResultsProperties( RunResult runResult, Properties results )
+    public void updateResultsProperties( RunResult runResult, Properties results )
     {
         results.setProperty( RESULTS_ERRORS, String.valueOf( runResult.getErrors() ) );
         results.setProperty( RESULTS_COMPLETED_COUNT, String.valueOf( runResult.getCompletedCount() ) );
@@ -103,37 +139,6 @@ public class SurefireStarter
         }
     }
 
-    public int runSuitesInProcess()
-        throws SurefireExecutionException
-    {
-        // TODO: replace with plexus
-
-        // The test classloader must be constructed first to avoid issues with commons-logging until we properly
-        // separate the TestNG classloader
-        ClassLoader testsClassLoader;
-        final ProviderConfiguration starterConfiguration = booterConfiguration.getSurefireStarterConfiguration();
-        final ClasspathConfiguration classpathConfiguration = starterConfiguration.getClasspathConfiguration();
-        String testClassPath = classpathConfiguration.getTestClasspath().getClassPathAsString();
-        System.setProperty( "surefire.test.class.path", testClassPath );
-        if ( starterConfiguration.isManifestOnlyJarRequestedAndUsable() )
-        {
-            testsClassLoader = getClass().getClassLoader(); // ClassLoader.getSystemClassLoader()
-            // SUREFIRE-459, trick the app under test into thinking its classpath was conventional
-            // (instead of a single manifest-only jar)
-            System.setProperty( "surefire.real.class.path", System.getProperty( "java.class.path" ) );
-            System.setProperty( "java.class.path", testClassPath );
-        }
-        else
-        {
-            testsClassLoader = classpathConfiguration.createTestClassLoader();
-        }
-
-        ClassLoader surefireClassLoader = classpathConfiguration.createSurefireClassLoader( testsClassLoader );
-
-        final RunResult runResult = invokeProvider( null, testsClassLoader, surefireClassLoader );
-        return processRunCount( runResult );
-    }
-
     /**
      * Returns the process return code based on the RunResult
      *
@@ -141,7 +146,7 @@ public class SurefireStarter
      * @return The process result code
      * @throws SurefireExecutionException When an exception is found
      */
-    private int processRunCount( RunResult runCount )
+    public int processRunCount( RunResult runCount )
         throws SurefireExecutionException
     {
 
@@ -152,5 +157,4 @@ public class SurefireStarter
 
         return runCount.getBooterCode();
     }
-
 }

Modified: maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SystemPropertyManager.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SystemPropertyManager.java?rev=1042451&r1=1042450&r2=1042451&view=diff
==============================================================================
--- maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SystemPropertyManager.java (original)
+++ maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SystemPropertyManager.java Sun Dec  5 21:19:24 2010
@@ -34,6 +34,7 @@ public class SystemPropertyManager
 
     /**
      * Loads the properties, closes the stream
+     *
      * @param inStream The stream to read from, will be closed
      * @return The properties
      * @throws java.io.IOException If something bad happens
@@ -52,7 +53,7 @@ public class SystemPropertyManager
             close( inStream );
         }
 
-        return new PropertiesWrapper( p);
+        return new PropertiesWrapper( p );
     }
 
     private static PropertiesWrapper loadProperties( File file )
@@ -69,7 +70,7 @@ public class SystemPropertyManager
         p.setAsSystemProperties();
     }
 
-    public File writePropertiesFile( Properties properties, File tempDirectory, String name, boolean deleteOnExit)
+    public File writePropertiesFile( Properties properties, File tempDirectory, String name, boolean deleteOnExit )
         throws IOException
     {
         File file = File.createTempFile( name, "tmp", tempDirectory );
@@ -83,7 +84,7 @@ public class SystemPropertyManager
         return file;
     }
 
-    void writePropertiesFile( File file, String name, Properties properties )
+    static void writePropertiesFile( File file, String name, Properties properties )
         throws IOException
     {
         FileOutputStream out = new FileOutputStream( file );
@@ -115,7 +116,7 @@ public class SystemPropertyManager
         {
             inputStream.close();
         }
-        catch( IOException ex )
+        catch ( IOException ex )
         {
             // ignore
         }