Github user nkronenfeld commented on a diff in the pull request:
https://github.com/apache/spark/pull/19529#discussion_r146311180
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/test/SQLTestUtils.scala ---
@@ -52,249 +36,23 @@ import org.apache.spark.util.{UninterruptibleThread,
Utils}
* Subclasses should *not* create `SQLContext`s in the test suite
constructor, which is
* prone to leaving multiple overlapping
[[org.apache.spark.SparkContext]]s in the same JVM.
*/
-private[sql] trait SQLTestUtils
- extends SparkFunSuite with Eventually
- with BeforeAndAfterAll
- with SQLTestData
- with PlanTest { self =>
-
- protected def sparkContext = spark.sparkContext
-
+private[sql] trait SQLTestUtils extends SparkFunSuite with
SQLTestUtilsBase with PlanTest {
--- End diff --
This has more changes - I left the data initialization stuff in
SQLTestUtils, as it wouldn't be needed by external tests. All the <'s below
are things that I pulled out of SQLTestUtilsBase and put back into SQLTestUtils
Running similarly:
git checkout 4a779bdac3e75c17b7d36c5a009ba6c948fa9fb6 SQLTestUtils.scala
diff SQLTestUtils.scala SQLTestUtilsBase.scala
I get
27d26
< import scala.util.control.NonFatal
30c29
< import org.scalatest.BeforeAndAfterAll
---
> import org.scalatest.{BeforeAndAfterAll, Suite}
33d31
< import org.apache.spark.SparkFunSuite
38c36
< import org.apache.spark.sql.catalyst.plans.PlanTest
---
> import org.apache.spark.sql.catalyst.plans.PlanTestBase
40d37
< import org.apache.spark.sql.catalyst.util._
43c40
< import org.apache.spark.util.{UninterruptibleThread, Utils}
---
> import org.apache.spark.util.Utils
46c43
< * Helper trait that should be extended by all SQL test suites.
---
> * Helper trait that can be extended by all external SQL test suites.
48,49c45
< * This allows subclasses to plugin a custom `SQLContext`. It comes
with test data
< * prepared in advance as well as all implicit conversions used
extensively by dataframes.
---
> * This allows subclasses to plugin a custom `SQLContext`.
55,56c51,52
< private[sql] trait SQLTestUtils
< extends SparkFunSuite with Eventually
---
> private[sql] trait SQLTestUtilsBase
> extends Eventually
59c55
< with PlanTest { self =>
---
> with PlanTestBase { self: Suite =>
63,65d58
< // Whether to materialize all test data before the first test is run
< private var loadTestDataBeforeTests = false
<
80,94d72
< /**
< * Materialize the test data immediately after the `SQLContext` is
set up.
< * This is necessary if the data is accessed by name but not
through direct reference.
< */
< protected def setupTestData(): Unit = {
< loadTestDataBeforeTests = true
< }
<
< protected override def beforeAll(): Unit = {
< super.beforeAll()
< if (loadTestDataBeforeTests) {
< loadTestData()
< }
< }
<
300,354d277
< /**
< * Disable stdout and stderr when running the test. To not output
the logs to the console,
< * ConsoleAppender's `follow` should be set to `true` so that it
will honors reassignments of
< * System.out or System.err. Otherwise, ConsoleAppender will still
output to the console even if
< * we change System.out and System.err.
< */
< protected def testQuietly(name: String)(f: => Unit): Unit = {
< test(name) {
< quietly {
< f
< }
< }
< }
<
< /**
< * Run a test on a separate `UninterruptibleThread`.
< */
< protected def testWithUninterruptibleThread(name: String, quietly:
Boolean = false)
< (body: => Unit): Unit = {
< val timeoutMillis = 10000
< @transient var ex: Throwable = null
<
< def runOnThread(): Unit = {
< val thread = new UninterruptibleThread(s"Testing thread for
test $name") {
< override def run(): Unit = {
< try {
< body
< } catch {
< case NonFatal(e) =>
< ex = e
< }
< }
< }
< thread.setDaemon(true)
< thread.start()
< thread.join(timeoutMillis)
< if (thread.isAlive) {
< thread.interrupt()
< // If this interrupt does not work, then this thread is most
likely running something that
< // is not interruptible. There is not much point to wait for
the thread to termniate, and
< // we rather let the JVM terminate the thread on exit.
< fail(
< s"Test '$name' running on o.a.s.util.UninterruptibleThread
timed out after" +
< s" $timeoutMillis ms")
< } else if (ex != null) {
< throw ex
< }
< }
<
< if (quietly) {
< testQuietly(name) { runOnThread() }
< } else {
< test(name) { runOnThread() }
< }
< }
365,407d287
< }
< }
<
< private[sql] object SQLTestUtils {
<
< def compareAnswers(
< sparkAnswer: Seq[Row],
< expectedAnswer: Seq[Row],
< sort: Boolean): Option[String] = {
< def prepareAnswer(answer: Seq[Row]): Seq[Row] = {
< // Converts data to types that we can do equality comparison
using Scala collections.
< // For BigDecimal type, the Scala type has a better definition
of equality test (similar to
< // Java's java.math.BigDecimal.compareTo).
< // For binary arrays, we convert it to Seq to avoid of calling
java.util.Arrays.equals for
< // equality test.
< // This function is copied from Catalyst's QueryTest
< val converted: Seq[Row] = answer.map { s =>
< Row.fromSeq(s.toSeq.map {
< case d: java.math.BigDecimal => BigDecimal(d)
< case b: Array[Byte] => b.toSeq
< case o => o
< })
< }
< if (sort) {
< converted.sortBy(_.toString())
< } else {
< converted
< }
< }
< if (prepareAnswer(expectedAnswer) != prepareAnswer(sparkAnswer)) {
< val errorMessage =
< s"""
< | == Results ==
< | ${sideBySide(
< s"== Expected Answer - ${expectedAnswer.size} ==" +:
< prepareAnswer(expectedAnswer).map(_.toString()),
< s"== Actual Answer - ${sparkAnswer.size} ==" +:
<
prepareAnswer(sparkAnswer).map(_.toString())).mkString("\n")}
< """.stripMargin
< Some(errorMessage)
< } else {
< None
< }
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
