From 3021037214726d2752eaa780e52b86b13d9b80b6 Mon Sep 17 00:00:00 2001 From: tedyu Date: Mon, 9 Nov 2015 18:44:52 -0800 Subject: [PATCH 1/7] Drop @VisibleForTesting annotation from QueryExecution --- .../scala/org/apache/spark/sql/execution/QueryExecution.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala index c2142d03f422b..77843f53b9bd0 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala @@ -17,8 +17,6 @@ package org.apache.spark.sql.execution -import com.google.common.annotations.VisibleForTesting - import org.apache.spark.rdd.RDD import org.apache.spark.sql.SQLContext import org.apache.spark.sql.catalyst.InternalRow @@ -33,7 +31,6 @@ import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan */ class QueryExecution(val sqlContext: SQLContext, val logical: LogicalPlan) { - @VisibleForTesting def assertAnalyzed(): Unit = sqlContext.analyzer.checkAnalysis(analyzed) lazy val analyzed: LogicalPlan = sqlContext.analyzer.execute(logical) From 3548fb84beb2e7b007f073c44c48f05210dd5462 Mon Sep 17 00:00:00 2001 From: tedyu Date: Mon, 9 Nov 2015 19:26:39 -0800 Subject: [PATCH 2/7] Drop @VisibleForTesting from two more classes --- .../scala/org/apache/spark/ui/jobs/JobProgressListener.scala | 2 -- .../org/apache/spark/network/shuffle/ShuffleTestAccessor.scala | 1 - 2 files changed, 3 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/JobProgressListener.scala b/core/src/main/scala/org/apache/spark/ui/jobs/JobProgressListener.scala index 77d034fa5ba2c..ca37829216f22 100644 --- a/core/src/main/scala/org/apache/spark/ui/jobs/JobProgressListener.scala +++ b/core/src/main/scala/org/apache/spark/ui/jobs/JobProgressListener.scala @@ -21,8 +21,6 @@ import java.util.concurrent.TimeoutException import scala.collection.mutable.{HashMap, HashSet, ListBuffer} -import com.google.common.annotations.VisibleForTesting - import org.apache.spark._ import org.apache.spark.annotation.DeveloperApi import org.apache.spark.executor.TaskMetrics diff --git a/yarn/src/test/scala/org/apache/spark/network/shuffle/ShuffleTestAccessor.scala b/yarn/src/test/scala/org/apache/spark/network/shuffle/ShuffleTestAccessor.scala index aa46ec5100f0e..94bf579dc8247 100644 --- a/yarn/src/test/scala/org/apache/spark/network/shuffle/ShuffleTestAccessor.scala +++ b/yarn/src/test/scala/org/apache/spark/network/shuffle/ShuffleTestAccessor.scala @@ -19,7 +19,6 @@ package org.apache.spark.network.shuffle import java.io.{IOException, File} import java.util.concurrent.ConcurrentMap -import com.google.common.annotations.VisibleForTesting import org.apache.hadoop.yarn.api.records.ApplicationId import org.fusesource.leveldbjni.JniDBFactory import org.iq80.leveldb.{DB, Options} From 62f3bc72b71e2f5787641ef521f6f3640dbb6b05 Mon Sep 17 00:00:00 2001 From: tedyu Date: Tue, 10 Nov 2015 06:30:25 -0800 Subject: [PATCH 3/7] Drop @VisibleForTesting from 3 more classes --- core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala | 4 +--- .../org/apache/spark/util/AsynchronousListenerBus.scala | 3 +-- .../org/apache/spark/util/collection/ExternalSorter.scala | 3 +-- scalastyle-config.xml | 7 +++++++ 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala b/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala index c72b588db57fe..8ce4bc9083e89 100644 --- a/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala +++ b/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala @@ -21,8 +21,6 @@ import javax.annotation.concurrent.GuardedBy import scala.util.control.NonFatal -import com.google.common.annotations.VisibleForTesting - import org.apache.spark.{Logging, SparkException} import org.apache.spark.rpc.{RpcAddress, RpcEndpoint, ThreadSafeRpcEndpoint} @@ -194,7 +192,7 @@ private[netty] class Inbox( def isEmpty: Boolean = inbox.synchronized { messages.isEmpty } /** Called when we are dropping a message. Test cases override this to test message dropping. */ - @VisibleForTesting + // VisibleForTesting protected def onDrop(message: InboxMessage): Unit = { logWarning(s"Drop $message because $endpointRef is stopped") } diff --git a/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala b/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala index 61b5a4cecddce..3225d44f23995 100644 --- a/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala +++ b/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala @@ -20,7 +20,6 @@ package org.apache.spark.util import java.util.concurrent._ import java.util.concurrent.atomic.AtomicBoolean -import com.google.common.annotations.VisibleForTesting import org.apache.spark.SparkContext /** @@ -123,7 +122,7 @@ private[spark] abstract class AsynchronousListenerBus[L <: AnyRef, E](name: Stri * time has elapsed. Throw `TimeoutException` if the specified time elapsed before the queue * emptied. */ - @VisibleForTesting + // VisibleForTesting @throws(classOf[TimeoutException]) def waitUntilEmpty(timeoutMillis: Long): Unit = { val finishTime = System.currentTimeMillis + timeoutMillis diff --git a/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala b/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala index a44e72b7c16d3..9e95db746cda7 100644 --- a/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala +++ b/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala @@ -23,7 +23,6 @@ import java.util.Comparator import scala.collection.mutable.ArrayBuffer import scala.collection.mutable -import com.google.common.annotations.VisibleForTesting import com.google.common.io.ByteStreams import org.apache.spark._ @@ -609,7 +608,7 @@ private[spark] class ExternalSorter[K, V, C]( * For now, we just merge all the spilled files in once pass, but this can be modified to * support hierarchical merging. */ - @VisibleForTesting + // VisibleForTesting def partitionedIterator: Iterator[(Int, Iterator[Product2[K, C]])] = { val usingMap = aggregator.isDefined val collection: WritablePartitionedPairCollection[K, C] = if (usingMap) map else buffer diff --git a/scalastyle-config.xml b/scalastyle-config.xml index 64a0c71bbef2a..cccdffc714c6f 100644 --- a/scalastyle-config.xml +++ b/scalastyle-config.xml @@ -150,6 +150,13 @@ This file is divided into 3 sections: // scalastyle:on println]]> + + @VisibleForTesting + + + Class\.forName Date: Tue, 10 Nov 2015 07:08:23 -0800 Subject: [PATCH 4/7] Missed one @VisibleForTesting in AsynchronousListenerBus.scala --- .../scala/org/apache/spark/util/AsynchronousListenerBus.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala b/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala index 3225d44f23995..315b5870db557 100644 --- a/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala +++ b/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala @@ -140,7 +140,7 @@ private[spark] abstract class AsynchronousListenerBus[L <: AnyRef, E](name: Stri /** * For testing only. Return whether the listener daemon thread is still alive. */ - @VisibleForTesting + // VisibleForTesting def listenerThreadIsAlive: Boolean = listenerThread.isAlive /** From dacec5e6b5f1d705ae9216f20f51f811a4ca70db Mon Sep 17 00:00:00 2001 From: tedyu Date: Tue, 10 Nov 2015 09:57:01 -0800 Subject: [PATCH 5/7] Address Andrew's comments --- core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala | 4 ++-- .../scala/org/apache/spark/util/AsynchronousListenerBus.scala | 4 ++-- .../org/apache/spark/util/collection/ExternalSorter.scala | 2 +- scalastyle-config.xml | 3 ++- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala b/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala index 8ce4bc9083e89..a34888a90d920 100644 --- a/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala +++ b/core/src/main/scala/org/apache/spark/rpc/netty/Inbox.scala @@ -191,8 +191,8 @@ private[netty] class Inbox( def isEmpty: Boolean = inbox.synchronized { messages.isEmpty } - /** Called when we are dropping a message. Test cases override this to test message dropping. */ - // VisibleForTesting + /** Called when we are dropping a message. Test cases override this to test message dropping. + * Exposed for testing. */ protected def onDrop(message: InboxMessage): Unit = { logWarning(s"Drop $message because $endpointRef is stopped") } diff --git a/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala b/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala index 315b5870db557..c20627b056bef 100644 --- a/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala +++ b/core/src/main/scala/org/apache/spark/util/AsynchronousListenerBus.scala @@ -121,8 +121,8 @@ private[spark] abstract class AsynchronousListenerBus[L <: AnyRef, E](name: Stri * For testing only. Wait until there are no more events in the queue, or until the specified * time has elapsed. Throw `TimeoutException` if the specified time elapsed before the queue * emptied. + * Exposed for testing. */ - // VisibleForTesting @throws(classOf[TimeoutException]) def waitUntilEmpty(timeoutMillis: Long): Unit = { val finishTime = System.currentTimeMillis + timeoutMillis @@ -139,8 +139,8 @@ private[spark] abstract class AsynchronousListenerBus[L <: AnyRef, E](name: Stri /** * For testing only. Return whether the listener daemon thread is still alive. + * Exposed for testing. */ - // VisibleForTesting def listenerThreadIsAlive: Boolean = listenerThread.isAlive /** diff --git a/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala b/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala index 9e95db746cda7..bd6844d045cad 100644 --- a/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala +++ b/core/src/main/scala/org/apache/spark/util/collection/ExternalSorter.scala @@ -607,8 +607,8 @@ private[spark] class ExternalSorter[K, V, C]( * * For now, we just merge all the spilled files in once pass, but this can be modified to * support hierarchical merging. + * Exposed for testing. */ - // VisibleForTesting def partitionedIterator: Iterator[(Int, Iterator[Product2[K, C]])] = { val usingMap = aggregator.isDefined val collection: WritablePartitionedPairCollection[K, C] = if (usingMap) map else buffer diff --git a/scalastyle-config.xml b/scalastyle-config.xml index cccdffc714c6f..de25efd5f158d 100644 --- a/scalastyle-config.xml +++ b/scalastyle-config.xml @@ -153,7 +153,8 @@ This file is divided into 3 sections: @VisibleForTesting From d5da5eae6dbc7be291264a80471e8eae436e07a6 Mon Sep 17 00:00:00 2001 From: tedyu Date: Tue, 10 Nov 2015 14:05:20 -0800 Subject: [PATCH 6/7] Correct indentation --- scalastyle-config.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scalastyle-config.xml b/scalastyle-config.xml index de25efd5f158d..6bf80834489a3 100644 --- a/scalastyle-config.xml +++ b/scalastyle-config.xml @@ -153,7 +153,7 @@ This file is divided into 3 sections: @VisibleForTesting From 29fe9e4aac0123470dbe5949837a32453691d41a Mon Sep 17 00:00:00 2001 From: tedyu Date: Tue, 10 Nov 2015 14:33:56 -0800 Subject: [PATCH 7/7] Address Andrew's comment --- scalastyle-config.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scalastyle-config.xml b/scalastyle-config.xml index 6bf80834489a3..050c3f360476f 100644 --- a/scalastyle-config.xml +++ b/scalastyle-config.xml @@ -153,8 +153,7 @@ This file is divided into 3 sections: @VisibleForTesting