This is an automated email from the ASF dual-hosted git repository.
sarutak 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 578105289d82 [SPARK-59825][SQL][TEST] Fix `HiveClientSuites` to work
with sbt `testOnly` and `-z` filtering
578105289d82 is described below
commit 578105289d82158f7d04d7976d547038d1a270c2
Author: Kousuke Saruta <[email protected]>
AuthorDate: Wed Sep 30 14:21:43 2026 +0900
[SPARK-59825][SQL][TEST] Fix `HiveClientSuites` to work with sbt `testOnly`
and `-z` filtering
### What changes were proposed in this pull request?
`HiveClientSuites` runs `HiveClientSuite` once per supported Hive metastore
(HMS) version by exposing them through ScalaTest's `nestedSuites`. This pattern
does not interoperate well with SBT's ScalaTest framework integration, and this
PR fixes two resulting problems:
1. `testOnly ... -- -z "<keyword>"` reports "No tests were executed"
because the ScalaTest SBT framework cannot propagate the test-name filter down
to the nested suites (their `suiteId`s differ from the parent's).
2. During a full run, test names and version info for HMS versions other
than the first are not displayed properly.
The fix works around a known ScalaTest limitation
(https://github.com/scalatest/scalatest/issues/2375):
- `HiveClientSuites`
- Override `testNames` to include the nested suites' test names so that
the SBT framework's `-z` substring matching can see them.
- Override `run` to handle both filtered (`-z`/`-t`) and full runs. The
SBT ScalaTest framework always calls `run` with `testName == None` and encodes
any selection as a `Selected` tag plus per-test `dynaTags` keyed by this
suite's `suiteId`; because our `testNames` exposes the nested names, those
selections land in this suite's`dynaTags`. The override runs our own tests
separately (filtered against`super.testNames`), then re-keys the matching
`dynaTags` to each nested suite's`suite [...]
- Factor the per-suite `SuiteStarting`/`SuiteCompleted`/`SuiteAborted`
reporting into a private `runNestedSuiteWithEvents` helper (needed because each
nested suite must receive its own re-keyed `Filter`, so `runNestedSuites`
cannot be used directly).
- Make `nestedSuites` a `lazy val` so the nested suites are not
re-created on every call.
- `HiveVersionSuite`
- Override `suiteId` to `s"${getClass.getName}($version)"` so each
version's nested suite has a unique `suiteId` (required for the `dynaTags`
redistribution above).
- `HiveClientSuite`
- Move client construction/teardown from the `"create client"` test body
into `beforeAll`/`afterAll`, so that running a single test via `-z` still
initializes `client`/`versionSpark`. The `"create client"` test now asserts
they were initialized in `beforeAll`.
Note on scope: the nested `HiveClientSuite` tests are order-dependent --
they share a single `client`/`versionSpark` and many tests rely on state
(databases, tables, partitions, functions) created by earlier tests. Selecting
a single order-dependent test with `-z` runs it in isolation and may fail
because its prerequisites were filtered out; independent tests (e.g. `"create
client"`) can be selected safely. This is a pre-existing property of these
tests and is called out in a code com [...]
### Why are the changes needed?
Developers cannot currently run a single Hive client test (or a subset) via
SBT's `-z` filter -- the run silently executes nothing, which is easy to
misread as "passed". This makes iterating on Hive client changes slow and
error-prone. The full-run display issue also hides which HMS versions actually
ran.
### Does this PR introduce _any_ user-facing change?
No. This only affects test-suite execution/reporting for `HiveClientSuites`.
### How was this patch tested?
Existing tests, plus manual verification against `master` and this branch
(JDK 17, `build/sbt`):
- Reproduced the bug on `master`:
```
build/sbt 'hive/testOnly
org.apache.spark.sql.hive.client.HiveClientSuites \
-- -z "should pick right user name as owner"'
```
-> `Total number of tests run: 0` / `No tests were executed.`
- On this branch, the same command runs the selected test across all HMS
versions:
-> `Total number of tests run: 8` / `succeeded 8, failed 0` / `All tests
passed.`
- Full run on this branch is unaffected:
```
build/sbt 'hive/testOnly
org.apache.spark.sql.hive.client.HiveClientSuites'
```
-> `595 succeeded, 0 failed` and test names for all HMS versions (2.0 /
2.1 / 2.2 / 2.3 / 3.0 / 3.1 / 4.0 / 4.1) are displayed (problem 2 fixed), with
no double execution.
- After the review-driven cleanup (removing the unreachable `runTest`
override, extracting `runNestedSuiteWithEvents`, switching to `NonFatal`),
re-verified that behavior is unchanged: `hive/Test/compile` succeeds, `-z`
selection still runs the selected test across all 8 HMS versions (8 passed, one
`beforeAll` per version), the full run is still `595 succeeded, 0 failed` with
all versions listed, and `hive/scalastyle` reports 0 errors.
Note: order-dependent tests in `HiveClientSuite` (e.g. `getTablesByName
when multiple tables`, which relies on the earlier `createTable` test) will
fail if selected alone with `-z`. That is a pre-existing property of these
tests, out of scope here, and is tracked separately.
### Was this patch authored or co-authored using generative AI tooling?
Kiro CLI/ Claude
Closes #59098 from sarutak/fix-hive-client-suites-sbt-test-filtering.
Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
---
.../spark/sql/hive/client/HiveClientSuite.scala | 30 ++++-
.../spark/sql/hive/client/HiveClientSuites.scala | 141 ++++++++++++++++++++-
.../spark/sql/hive/client/HiveVersionSuite.scala | 2 +
3 files changed, 164 insertions(+), 9 deletions(-)
diff --git
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuite.scala
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuite.scala
index 0eebdc9006db..bb701d80bea0 100644
---
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuite.scala
+++
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuite.scala
@@ -49,6 +49,27 @@ class HiveClientSuite(version: String) extends
HiveVersionSuite(version) {
private val ver = IsolatedClientLoader.hiveVersion(version)
+ override def beforeAll(): Unit = {
+ super.beforeAll()
+ System.gc() // Hack to avoid SEGV on some JVM versions.
+ val hadoopConf = new Configuration()
+ hadoopConf.set("test", "success")
+ client = buildClient(hadoopConf)
+ versionSpark = TestHiveVersion(client)
+ }
+
+ override def afterAll(): Unit = {
+ try {
+ if (versionSpark != null) {
+ versionSpark.reset()
+ versionSpark = null
+ }
+ client = null
+ } finally {
+ super.afterAll()
+ }
+ }
+
/**
* Drops table `tableName` after calling `f`.
*/
@@ -61,13 +82,8 @@ class HiveClientSuite(version: String) extends
HiveVersionSuite(version) {
}
test("create client") {
- client = null
- System.gc() // Hack to avoid SEGV on some JVM versions.
- val hadoopConf = new Configuration()
- hadoopConf.set("test", "success")
- client = buildClient(hadoopConf)
- if (versionSpark != null) versionSpark.reset()
- versionSpark = TestHiveVersion(client)
+ assert(client != null, "client should be initialized in beforeAll")
+ assert(versionSpark != null, "versionSpark should be initialized in
beforeAll")
assert(versionSpark.sharedState.externalCatalog.unwrapped.asInstanceOf[HiveExternalCatalog]
.client.version.fullVersion.startsWith(version))
}
diff --git
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuites.scala
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuites.scala
index e43308f62a49..8b4270573ef7 100644
---
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuites.scala
+++
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientSuites.scala
@@ -19,8 +19,10 @@ package org.apache.spark.sql.hive.client
import java.net.URI
+import scala.util.control.NonFatal
+
import org.apache.hadoop.conf.Configuration
-import org.scalatest.Suite
+import org.scalatest.{Args, CompositeStatus, DynaTags, FailedStatus, Filter,
Status, Suite}
import org.apache.spark.SparkFunSuite
import org.apache.spark.sql.catalyst.catalog.CatalogDatabase
@@ -90,7 +92,142 @@ class HiveClientSuites extends SparkFunSuite with
HiveClientVersions {
assert(getNestedMessages(e) contains "Unknown column 'A0.OWNER_NAME' in
'field list'")
}
- override def nestedSuites: IndexedSeq[Suite] = {
+ // Lazily initialize nested suites to avoid re-creating them on every call
+ private lazy val versionedSuites: IndexedSeq[HiveClientSuite] = {
versions.map(new HiveClientSuite(_))
}
+
+ override def nestedSuites: IndexedSeq[Suite] = versionedSuites
+
+ // Include nested suite test names in this suite's testNames so that SBT's
+ // `-z "keyword"` filtering can find them. Without this, the ScalaTest SBT
framework
+ // would skip the entire suite when no parent-level test matches the keyword.
+ override def testNames: Set[String] = {
+ super.testNames ++ versionedSuites.flatMap(_.testNames)
+ }
+
+ // Override run to handle both filtered and full test runs correctly.
+ //
+ // The ScalaTest SBT framework always invokes a suite's run with testName ==
None and
+ // encodes any `-z` / `-t` selection as a "Selected" tag plus per-test
dynaTags keyed by
+ // the suite's suiteId (see the workaround for
+ // https://github.com/scalatest/scalatest/issues/2375). Because our
testNames override
+ // exposes the nested suites' test names, those selections land in *this*
suite's dynaTags
+ // rather than the nested suites'. This override redistributes them:
+ // - run our own tests via super.runTest (filtered against super.testNames
only), then
+ // - for each nested suite, re-key the matching dynaTags to that nested
suite's suiteId
+ // and run it once.
+ // Own tests and nested-suite tests are dispatched separately so a full run
never
+ // double-executes (our testNames intentionally includes the nested names).
+ //
+ // NOTE: the nested HiveClientSuite tests are order-dependent (they share a
single
+ // `client`/`versionSpark` built in beforeAll, and e.g.
table/partition/function tests
+ // rely on state created by earlier tests). Selecting a single
order-dependent test with
+ // `-z` will run it in isolation and may fail because its prerequisites were
filtered out.
+ // Independent tests (e.g. "create client") can be selected safely. Making
every test
+ // self-contained is out of scope for this change.
+ override def run(testName: Option[String], args: Args): Status = {
+ testName match {
+ case Some(_) =>
+ // Not exercised by the SBT framework (it uses testName == None +
Selected tag, see
+ // above); delegate to the standard implementation as a fallback for
other runners.
+ super.run(testName, args)
+ case None =>
+ // Run our own tests only (filter against super.testNames, not the
nested names).
+ val ownTests = super.testNames
+ val statusBuffer = new scala.collection.mutable.ListBuffer[Status]()
+ for ((tn, ignoreTest) <- args.filter(ownTests, tags, suiteId)) {
+ if (!args.stopper.stopRequested) {
+ if (!ignoreTest) {
+ statusBuffer += super.runTest(tn, args)
+ }
+ }
+ }
+ val testsStatus = new CompositeStatus(Set.empty ++ statusBuffer)
+
+ // Then run the nested suites. When a `-z`/`-t` selection is active
the SBT framework
+ // sets tagsToInclude = Some(Set("Selected")) with dynaTags keyed by
THIS suite's
+ // suiteId; we redistribute those tags to each nested suite by its own
suiteId.
+ val selectedTag = "org.scalatest.Selected"
+ val isSelectedFiltering =
args.filter.tagsToInclude.exists(_.contains(selectedTag))
+
+ val nestedSuitesStatus = if (isSelectedFiltering) {
+ val myTestTags = args.filter.dynaTags.testTags.getOrElse(suiteId,
Map.empty)
+ val nestedStatusBuffer = new
scala.collection.mutable.ListBuffer[Status]()
+
+ for (nestedSuite <- nestedSuites) {
+ if (!args.stopper.stopRequested) {
+ // Find which tests in this nested suite were "Selected" in our
dynaTags.
+ val nestedTestNames = nestedSuite.testNames
+ val selectedForNested = myTestTags.filter {
+ case (testName, tagSet) =>
+ tagSet.contains(selectedTag) &&
nestedTestNames.contains(testName)
+ }
+ if (selectedForNested.nonEmpty) {
+ val nestedFilter = Filter(
+ tagsToInclude = Some(Set(selectedTag)),
+ args.filter.tagsToExclude,
+ excludeNestedSuites = false,
+ dynaTags = DynaTags(Map.empty, Map(nestedSuite.suiteId ->
selectedForNested))
+ )
+ nestedStatusBuffer +=
+ runNestedSuiteWithEvents(nestedSuite, args.copy(filter =
nestedFilter), args)
+ }
+ }
+ }
+ new CompositeStatus(Set.empty ++ nestedStatusBuffer)
+ } else {
+ // No `-z`/`-t` selection: run all nested suites normally via
runNestedSuites,
+ // which handles SuiteStarting/SuiteCompleted event reporting itself.
+ val nestedFilter = Filter(
+ tagsToInclude = None,
+ args.filter.tagsToExclude,
+ excludeNestedSuites = false
+ )
+ runNestedSuites(args.copy(filter = nestedFilter))
+ }
+ new CompositeStatus(Set(testsStatus, nestedSuitesStatus))
+ }
+ }
+
+ // Run a single nested suite while emitting the
SuiteStarting/SuiteCompleted/SuiteAborted
+ // events that runNestedSuites would normally emit. We cannot use
runNestedSuites directly
+ // here because we need to pass a per-suite Filter (with re-keyed dynaTags)
to each nested
+ // suite. `runArgs` carries the reporter/tracker; `nestedArgs` carries the
per-suite filter.
+ private def runNestedSuiteWithEvents(
+ nestedSuite: Suite, nestedArgs: Args, runArgs: Args): Status = {
+ import org.scalatest.events._
+ val report = runArgs.reporter
+ val tracker = runArgs.tracker
+ val suiteClassName = nestedSuite.getClass.getName
+ val suiteStartTime = System.currentTimeMillis
+ report(SuiteStarting(tracker.nextOrdinal(), nestedSuite.suiteName,
nestedSuite.suiteId,
+ Some(suiteClassName), None, Some(TopOfClass(suiteClassName)),
nestedSuite.rerunner))
+ try {
+ val status = nestedSuite.run(None, nestedArgs)
+ val duration = System.currentTimeMillis - suiteStartTime
+ status.unreportedException match {
+ case Some(ue) =>
+ report(SuiteAborted(tracker.nextOrdinal(), ue.getMessage,
nestedSuite.suiteName,
+ nestedSuite.suiteId, Some(suiteClassName), Some(ue),
Some(duration),
+ None, Some(SeeStackDepthException), nestedSuite.rerunner))
+ FailedStatus
+ case None =>
+ report(SuiteCompleted(tracker.nextOrdinal(), nestedSuite.suiteName,
+ nestedSuite.suiteId, Some(suiteClassName), Some(duration), None,
+ Some(TopOfClass(suiteClassName)), nestedSuite.rerunner))
+ status
+ }
+ } catch {
+ case NonFatal(e) =>
+ val eMessage = e.getMessage
+ val rawString = if (eMessage != null && eMessage.nonEmpty) eMessage
+ else e.getClass.getName
+ val duration = System.currentTimeMillis - suiteStartTime
+ report(SuiteAborted(tracker.nextOrdinal(), rawString,
nestedSuite.suiteName,
+ nestedSuite.suiteId, Some(suiteClassName), Some(e), Some(duration),
+ None, Some(SeeStackDepthException), nestedSuite.rerunner))
+ FailedStatus
+ }
+ }
}
diff --git
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveVersionSuite.scala
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveVersionSuite.scala
index 9866ce58775e..ba8d22f69389 100644
---
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveVersionSuite.scala
+++
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveVersionSuite.scala
@@ -45,6 +45,8 @@ private[client] abstract class HiveVersionSuite(version:
String) extends SparkFu
override def suiteName: String = s"${super.suiteName}($version)"
+ override val suiteId: String = s"${getClass.getName}($version)"
+
override protected def test(testName: String, testTags: Tag*)(testFun: =>
Any)
(implicit pos: Position): Unit = {
super.test(s"$version: $testName", testTags: _*)(testFun)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
