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 2019/05/23 16:06:57 UTC

[maven-surefire] 01/03: refactoring

This is an automated email from the ASF dual-hosted git repository.

tibordigana pushed a commit to branch 1546-1222-v2
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git

commit fd2f7fd8a2676303b2feaede3f7af032e023f723
Author: tibordigana <ti...@apache.org>
AuthorDate: Thu May 23 01:24:37 2019 +0200

    refactoring
---
 .../output/ThreadedStreamConsumer.java             |  2 +-
 .../surefire/report/DefaultReporterFactory.java    |  3 +-
 .../maven/surefire/report/RunStatistics.java       | 21 +------
 .../plugin/surefire/SurefireReflectorTest.java     | 72 +++++++++++++++++++++-
 .../report/DefaultReporterFactoryTest.java         | 11 ++--
 .../maven/surefire/booter/SurefireReflector.java   | 49 +++------------
 6 files changed, 88 insertions(+), 70 deletions(-)

diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java
index dad88fd..388f16d 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java
@@ -40,7 +40,7 @@ public final class ThreadedStreamConsumer
 {
     private static final String END_ITEM = "";
 
-    private static final int ITEM_LIMIT_BEFORE_SLEEP = 10 * 1000;
+    private static final int ITEM_LIMIT_BEFORE_SLEEP = 10_000;
 
     private final BlockingQueue<String> items = new ArrayBlockingQueue<>( ITEM_LIMIT_BEFORE_SLEEP );
 
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 d51b3e9..0d7ca78 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
@@ -272,7 +272,7 @@ public class DefaultReporterFactory
      * Merge all the TestMethodStats in each TestRunListeners and put results into flakyTests, failedTests and
      * errorTests, indexed by test class and method name. Update globalStatistics based on the result of the merge.
      */
-    void mergeTestHistoryResult()
+    private void mergeTestHistoryResult()
     {
         globalStats = new RunStatistics();
         flakyTests = new TreeMap<>();
@@ -388,7 +388,6 @@ public class DefaultReporterFactory
 
         for ( Map.Entry<String, List<TestMethodStats>> entry : testStats.entrySet() )
         {
-            printed = true;
             List<TestMethodStats> testMethodStats = entry.getValue();
             if ( testMethodStats.size() == 1 )
             {
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/surefire/report/RunStatistics.java b/maven-surefire-common/src/main/java/org/apache/maven/surefire/report/RunStatistics.java
index 29aa8dc..7c911ec 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/surefire/report/RunStatistics.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/surefire/report/RunStatistics.java
@@ -19,13 +19,12 @@ package org.apache.maven.surefire.report;
  * under the License.
  */
 
-import org.apache.maven.plugin.surefire.report.TestSetStats;
 import org.apache.maven.surefire.suite.RunResult;
 
 /**
  * @author Kristian Rosenvold
  */
-public class RunStatistics
+public final class RunStatistics
 {
     private int completedCount;
 
@@ -37,16 +36,6 @@ public class RunStatistics
 
     private int flakes;
 
-    public synchronized boolean hadFailures()
-    {
-        return failures > 0;
-    }
-
-    public synchronized boolean hadErrors()
-    {
-        return errors > 0;
-    }
-
     public synchronized int getCompletedCount()
     {
         return completedCount;
@@ -72,14 +61,6 @@ public class RunStatistics
         return flakes;
     }
 
-    public synchronized void add( TestSetStats testSetStats )
-    {
-        this.completedCount += testSetStats.getCompletedCount();
-        this.errors += testSetStats.getErrors();
-        this.failures += testSetStats.getFailures();
-        this.skipped += testSetStats.getSkipped();
-    }
-
     public synchronized void set( int completedCount, int errors, int failures, int skipped, int flakes )
     {
         this.completedCount = completedCount;
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireReflectorTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireReflectorTest.java
index a7c8b9d..2553617 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireReflectorTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireReflectorTest.java
@@ -1 +1,71 @@
-package org.apache.maven.plugin.surefire;

/*
 * 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.plugin.surefire.log.api.ConsoleLogger;
import org.apache.maven.plugin.surefire.log.api.ConsoleLoggerDecorator;
import o
 rg.apache.maven.plugin.surefire.log.api.PrintStreamLogger;
import org.apache.maven.surefire.booter.IsolatedClassLoader;
import org.apache.maven.surefire.booter.SurefireReflector;
import org.junit.Before;
import org.junit.Test;

import static org.apache.maven.surefire.util.ReflectionUtils.getMethod;
import static org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.sameInstance;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

/**
 * @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
 * @see ConsoleLogger
 * @see SurefireReflector
 * @since 2.20
 */
public class SurefireReflectorTest
{
  
   private ConsoleLogger logger;
    private SurefireReflector reflector;

    @Before
    public void prepareData()
    {
        logger = spy( new PrintStreamLogger( System.out ) );
        ClassLoader cl = new IsolatedClassLoader( Thread.currentThread().getContextClassLoader(), false, "role" );
        reflector = new SurefireReflector( cl );
    }

    @Test
    public void shouldProxyConsoleLogger()
    {
        Object mirror = reflector.createConsoleLogger( logger );
        assertThat( mirror, is( notNullValue() ) );
        assertThat( mirror.getClass().getInterfaces()[0].getName(), is( ConsoleLogger.class.getName() ) );
        assertThat( mirror, is( not( sameInstance( (Object) logger ) ) ) );
        assertThat( mirror, is( instanceOf( ConsoleLoggerDecorator.class ) ) );
        invokeMethodWithArray( mirror, getMethod( mirror, "info", String.class ), "Hi There!" );
        verify( logger, times( 1 ) ).info( "Hi There!" );
    }
}
\ No newline at end of file
+package org.apache.maven.plugin.surefire;
+
+/*
+ * 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.plugin.surefire.log.api.ConsoleLogger;
+import org.apache.maven.plugin.surefire.log.api.ConsoleLoggerDecorator;
+import org.apache.maven.plugin.surefire.log.api.PrintStreamLogger;
+import org.apache.maven.surefire.booter.IsolatedClassLoader;
+import org.apache.maven.surefire.booter.SurefireReflector;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.apache.maven.surefire.util.ReflectionUtils.getMethod;
+import static org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.sameInstance;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+/**
+ * @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
+ * @see ConsoleLogger
+ * @see SurefireReflector
+ * @since 2.20
+ */
+public class SurefireReflectorTest
+{
+    private ConsoleLogger logger;
+    private ClassLoader cl;
+
+    @Before
+    public void prepareData()
+    {
+        logger = spy( new PrintStreamLogger( System.out ) );
+        cl = new IsolatedClassLoader( Thread.currentThread().getContextClassLoader(), false, "role" );
+    }
+
+    @Test
+    public void shouldProxyConsoleLogger()
+    {
+        Object mirror = SurefireReflector.createConsoleLogger( logger, cl );
+        assertThat( mirror, is( notNullValue() ) );
+        assertThat( mirror.getClass().getInterfaces()[0].getName(), is( ConsoleLogger.class.getName() ) );
+        assertThat( mirror, is( not( sameInstance( (Object) logger ) ) ) );
+        assertThat( mirror, is( instanceOf( ConsoleLoggerDecorator.class ) ) );
+        invokeMethodWithArray( mirror, getMethod( mirror, "info", String.class ), "Hi There!" );
+        verify( logger, times( 1 ) ).info( "Hi There!" );
+    }
+}
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactoryTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactoryTest.java
index 55c3a64..4bdfe8b 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactoryTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactoryTest.java
@@ -35,6 +35,7 @@ import org.apache.maven.surefire.report.SafeThrowable;
 import org.apache.maven.surefire.report.StackTraceWriter;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
 import static org.apache.maven.plugin.surefire.report.DefaultReporterFactory.TestResultType.error;
 import static org.apache.maven.plugin.surefire.report.DefaultReporterFactory.TestResultType.failure;
 import static org.apache.maven.plugin.surefire.report.DefaultReporterFactory.TestResultType.flake;
@@ -44,6 +45,7 @@ import static org.apache.maven.plugin.surefire.report.DefaultReporterFactory.Tes
 import static org.apache.maven.plugin.surefire.report.DefaultReporterFactory.getTestResultType;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.powermock.reflect.Whitebox.invokeMethod;
 
 public class DefaultReporterFactoryTest
     extends TestCase
@@ -63,6 +65,7 @@ public class DefaultReporterFactoryTest
     private final static String ERROR = "error";
 
     public void testMergeTestHistoryResult()
+            throws Exception
     {
         MessageUtils.setColorEnabled( false );
         File reportsDirectory = new File("target");
@@ -111,7 +114,7 @@ public class DefaultReporterFactoryTest
         factory.addListener( secondRunListener );
         factory.addListener( thirdRunListener );
 
-        factory.mergeTestHistoryResult();
+        invokeMethod( factory, "mergeTestHistoryResult" );
         RunStatistics mergedStatistics = factory.getGlobalRunStatistics();
 
         // Only TEST_THREE is a failing test, other three are flaky tests
@@ -132,14 +135,12 @@ public class DefaultReporterFactoryTest
         reporter.reset();
         factory.printTestFailures( error );
         String[] expectedFailureOutput =
-            { "Errors: ", TEST_THREE, "  Run 1: " + ASSERTION_FAIL, "  Run 2: " + ERROR, "  Run 3: " + ERROR, ""
-            };
+            { "Errors: ", TEST_THREE, "  Run 1: " + ASSERTION_FAIL, "  Run 2: " + ERROR, "  Run 3: " + ERROR, "" };
         assertEquals( asList( expectedFailureOutput ), reporter.getMessages() );
 
         reporter.reset();
         factory.printTestFailures( failure );
-        String[] expectedErrorOutput = { };
-        assertEquals( asList( expectedErrorOutput ), reporter.getMessages() );
+        assertEquals( emptyList(), reporter.getMessages() );
     }
 
     static final class DummyTestReporter implements ConsoleLogger
diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/booter/SurefireReflector.java b/surefire-api/src/main/java/org/apache/maven/surefire/booter/SurefireReflector.java
index 360c802..5cc1415 100644
--- a/surefire-api/src/main/java/org/apache/maven/surefire/booter/SurefireReflector.java
+++ b/surefire-api/src/main/java/org/apache/maven/surefire/booter/SurefireReflector.java
@@ -34,10 +34,8 @@ import org.apache.maven.surefire.testset.TestRequest;
 import org.apache.maven.surefire.util.RunOrder;
 import org.apache.maven.surefire.util.SurefireReflectionException;
 
-import javax.annotation.Nonnull;
 import java.io.File;
 import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -147,32 +145,9 @@ public class SurefireReflector
         int getSkipped = (Integer) invokeGetter( result, "getSkipped" );
         int getFailures = (Integer) invokeGetter( result, "getFailures" );
         return new RunResult( getCompletedCount1, getErrors, getFailures, getSkipped );
-
-    }
-
-    class ClassLoaderProxy
-        implements InvocationHandler
-    {
-        private final Object target;
-
-        /**
-         * @param delegate a target
-         */
-        ClassLoaderProxy( Object delegate )
-        {
-            this.target = delegate;
-        }
-
-        @Override
-        public Object invoke( Object proxy, Method method, Object[] args )
-            throws Throwable
-        {
-            Method delegateMethod = target.getClass().getMethod( method.getName(), method.getParameterTypes() );
-            return delegateMethod.invoke( target, args );
-        }
     }
 
-    Object createTestRequest( TestRequest suiteDefinition )
+    private Object createTestRequest( TestRequest suiteDefinition )
     {
         if ( suiteDefinition == null )
         {
@@ -191,7 +166,7 @@ public class SurefireReflector
         }
     }
 
-    Object createTestListResolver( TestListResolver resolver )
+    private Object createTestListResolver( TestListResolver resolver )
     {
         if ( resolver == null )
         {
@@ -204,7 +179,7 @@ public class SurefireReflector
         }
     }
 
-    Object createDirectoryScannerParameters( DirectoryScannerParameters directoryScannerParameters )
+    private Object createDirectoryScannerParameters( DirectoryScannerParameters directoryScannerParameters )
     {
         if ( directoryScannerParameters == null )
         {
@@ -222,8 +197,7 @@ public class SurefireReflector
                             RunOrder.asString( directoryScannerParameters.getRunOrder() ) );
     }
 
-
-    Object createRunOrderParameters( RunOrderParameters runOrderParameters )
+    private Object createRunOrderParameters( RunOrderParameters runOrderParameters )
     {
         if ( runOrderParameters == null )
         {
@@ -236,7 +210,7 @@ public class SurefireReflector
         return newInstance( constructor, RunOrder.asString( runOrderParameters.getRunOrder() ), runStatisticsFile );
     }
 
-    Object createTestArtifactInfo( TestArtifactInfo testArtifactInfo )
+    private Object createTestArtifactInfo( TestArtifactInfo testArtifactInfo )
     {
         if ( testArtifactInfo == null )
         {
@@ -247,7 +221,7 @@ public class SurefireReflector
         return newInstance( constructor, testArtifactInfo.getVersion(), testArtifactInfo.getClassifier() );
     }
 
-    Object createReporterConfiguration( ReporterConfiguration reporterConfig )
+    private Object createReporterConfiguration( ReporterConfiguration reporterConfig )
     {
         Constructor constructor = getConstructor( reporterConfiguration, File.class, boolean.class );
         return newInstance( constructor, reporterConfig.getReportsDirectory(), reporterConfig.isTrimStackTrace() );
@@ -315,7 +289,7 @@ public class SurefireReflector
         invokeSetter( o, "setSystemExitTimeout", Integer.class, systemExitTimeout );
     }
 
-    public void setDirectoryScannerParameters( Object o, DirectoryScannerParameters dirScannerParams )
+    void setDirectoryScannerParameters( Object o, DirectoryScannerParameters dirScannerParams )
     {
         Object param = createDirectoryScannerParameters( dirScannerParams );
         invokeSetter( o, "setDirectoryScannerParameters", directoryScannerParameters, param );
@@ -362,8 +336,7 @@ public class SurefireReflector
         }
     }
 
-
-    void setReporterConfiguration( Object o, ReporterConfiguration reporterConfiguration )
+    private void setReporterConfiguration( Object o, ReporterConfiguration reporterConfiguration )
     {
         Object param = createReporterConfiguration( reporterConfiguration );
         invokeSetter( o, "setReporterConfiguration", this.reporterConfiguration, param );
@@ -402,11 +375,6 @@ public class SurefireReflector
         return runResult.isAssignableFrom( o.getClass() );
     }
 
-    public Object createConsoleLogger( @Nonnull ConsoleLogger consoleLogger )
-    {
-        return createConsoleLogger( consoleLogger, surefireClassLoader );
-    }
-
     private static Collection<Integer> toOrdinals( Collection<? extends Enum> enums )
     {
         Collection<Integer> ordinals = new ArrayList<>( enums.size() );
@@ -429,5 +397,4 @@ public class SurefireReflector
             throw new SurefireReflectionException( e );
         }
     }
-
 }