(spark) branch master updated: [SPARK-56595][SQL][TESTS] Move non-anyfun helpers from QueryTest to QueryTestBase

[ru...@apache.org]

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

zhengruifeng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new cefd6cab8ae5 [SPARK-56595][SQL][TESTS] Move non-anyfun helpers from QueryTest to QueryTestBase
cefd6cab8ae5 is described below

commit cefd6cab8ae57a580e68a87a6ff427a049b72d16
Author: Ruifeng Zheng <ru...@apache.org>
AuthorDate: Fri Apr 24 11:52:31 2026 +0800

    [SPARK-56595][SQL][TESTS] Move non-anyfun helpers from QueryTest to QueryTestBase
    
    ### What changes were proposed in this pull request?
    
    Move seven members from `trait QueryTest` down to `trait QueryTestBase`. None of them depend on `AnyFunSuite.test(...)`, so they fit in `QueryTestBase` (self-type `Suite`) without changing the self-type:
    
    - `private var loadTestDataBeforeTests`
    - `override def beforeAll()`
    - `setupTestData()`
    - `waitForTasksToFinish()`
    - `withTempPaths(numPaths)(f)`
    - `getCurrentClassCallSitePattern`
    - `getNextLineCallSitePattern(lines)`
    
    All dependencies (`spark` from `SQLTestData`, `eventually`/`timeout` from `Eventually`, `super.beforeAll()` via `BeforeAndAfterAll`, `loadTestData` from `SQLTestData`) are already parents of `QueryTestBase`, so this is a pure relocation — the `super.beforeAll()` chain resolves the same way at the concrete-class level.
    
    ### Why are the changes needed?
    
    Making these helpers available to `QueryTestBase` allows tests that mix in `QueryTestBase` directly (without the `AnyFunSuite` overhead, e.g., non-FunSuite test styles) to reuse them. No functional change today; this lowers the bar for downstream test-style flexibility.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No, test-only.
    
    ### How was this patch tested?
    
    Locally compiled `sql/Test/compile`. Ran `SortSuite` (255 tests) — all pass. No call-site churn; every existing caller still reaches these through `QueryTest` via inheritance.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Opus 4.7
    
    Closes #55509 from zhengruifeng/move-querytest-methods-to-querytestbase.
    
    Authored-by: Ruifeng Zheng <ru...@apache.org>
    Signed-off-by: Ruifeng Zheng <ru...@foxmail.com>
---
 .../scala/org/apache/spark/sql/QueryTest.scala     | 111 +++++++++++----------
 1 file changed, 56 insertions(+), 55 deletions(-)

diff --git a/sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala b/sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala
index 33160da53d3a..47ac9ea810a7 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala
@@ -624,6 +624,62 @@ trait QueryTestBase
     }
   }
 
+  // Whether to materialize all test data before the first test is run
+  private var loadTestDataBeforeTests = false
+
+  protected override def beforeAll(): Unit = {
+    super.beforeAll()
+    if (loadTestDataBeforeTests) {
+      loadTestData()
+    }
+  }
+
+  /**
+   * 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
+  }
+
+  /**
+   * Waits for all tasks on all executors to be finished.
+   */
+  protected def waitForTasksToFinish(): Unit = {
+    eventually(timeout(10.seconds)) {
+      assert(spark.sparkContext.statusTracker
+        .getExecutorInfos.map(_.numRunningTasks()).sum == 0)
+    }
+  }
+
+  /**
+   * Creates the specified number of temporary directories, which is then passed to `f` and will be
+   * deleted after `f` returns.
+   */
+  protected def withTempPaths(numPaths: Int)(f: Seq[File] => Unit): Unit = {
+    val files = Array.fill[File](numPaths)(Utils.createTempDir().getCanonicalFile)
+    try f(files.toImmutableArraySeq) finally {
+      // wait for all tasks to finish before deleting files
+      waitForTasksToFinish()
+      files.foreach(Utils.deleteRecursively)
+    }
+  }
+
+  protected def getCurrentClassCallSitePattern: String = {
+    val stack = Thread.currentThread().getStackTrace()
+    val idx = stack.lastIndexWhere(_.getMethodName == "getCurrentClassCallSitePattern")
+    val cs = stack(idx + 1)
+    s"${cs.getClassName}\\..*\\(${cs.getFileName}:\\d+\\)"
+  }
+
+  protected def getNextLineCallSitePattern(lines: Int = 1): String = {
+    val stack = Thread.currentThread().getStackTrace()
+    val idx = stack.lastIndexWhere(_.getMethodName == "getNextLineCallSitePattern")
+    val cs = stack(idx + 1)
+    Pattern.quote(
+      s"${cs.getClassName}.${cs.getMethodName}(${cs.getFileName}:${cs.getLineNumber + lines})")
+  }
+
 }
 
 /**
@@ -637,15 +693,6 @@ trait QueryTestBase
  * prone to leaving multiple overlapping [[org.apache.spark.SparkContext]]s in the same JVM.
  */
 trait QueryTest extends SparkFunSuite with QueryTestBase with PlanTest {
-  // Whether to materialize all test data before the first test is run
-  private var loadTestDataBeforeTests = false
-
-  protected override def beforeAll(): Unit = {
-    super.beforeAll()
-    if (loadTestDataBeforeTests) {
-      loadTestData()
-    }
-  }
 
   /**
    * Creates a temporary directory, which is then passed to `f` and will be deleted after `f`
@@ -672,14 +719,6 @@ trait QueryTest extends SparkFunSuite with QueryTestBase with PlanTest {
     }
   }
 
-  /**
-   * 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
-  }
-
   /**
    * 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 honor reassignments of
@@ -760,44 +799,6 @@ trait QueryTest extends SparkFunSuite with QueryTestBase with PlanTest {
       f(tmpFile)
     }
   }
-
-  /**
-   * Waits for all tasks on all executors to be finished.
-   */
-  protected def waitForTasksToFinish(): Unit = {
-    eventually(timeout(10.seconds)) {
-      assert(spark.sparkContext.statusTracker
-        .getExecutorInfos.map(_.numRunningTasks()).sum == 0)
-    }
-  }
-
-  /**
-   * Creates the specified number of temporary directories, which is then passed to `f` and will be
-   * deleted after `f` returns.
-   */
-  protected def withTempPaths(numPaths: Int)(f: Seq[File] => Unit): Unit = {
-    val files = Array.fill[File](numPaths)(Utils.createTempDir().getCanonicalFile)
-    try f(files.toImmutableArraySeq) finally {
-      // wait for all tasks to finish before deleting files
-      waitForTasksToFinish()
-      files.foreach(Utils.deleteRecursively)
-    }
-  }
-
-  protected def getCurrentClassCallSitePattern: String = {
-    val stack = Thread.currentThread().getStackTrace()
-    val idx = stack.lastIndexWhere(_.getMethodName == "getCurrentClassCallSitePattern")
-    val cs = stack(idx + 1)
-    s"${cs.getClassName}\\..*\\(${cs.getFileName}:\\d+\\)"
-  }
-
-  protected def getNextLineCallSitePattern(lines: Int = 1): String = {
-    val stack = Thread.currentThread().getStackTrace()
-    val idx = stack.lastIndexWhere(_.getMethodName == "getNextLineCallSitePattern")
-    val cs = stack(idx + 1)
-    Pattern.quote(
-      s"${cs.getClassName}.${cs.getMethodName}(${cs.getFileName}:${cs.getLineNumber + lines})")
-  }
 }
 
 object QueryTest extends Assertions {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org