This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new f7f8b8128d72 [SPARK-56505][SQL][TEST][FOLLOWUP] Make the classic
session in SparkSessionBinderBase an overridable seam
f7f8b8128d72 is described below
commit f7f8b8128d720a8203c257b03cd57197fd0eb5af
Author: Wenchen Fan <[email protected]>
AuthorDate: Wed Jul 8 00:42:57 2026 +0800
[SPARK-56505][SQL][TEST][FOLLOWUP] Make the classic session in
SparkSessionBinderBase an overridable seam
### What changes were proposed in this pull request?
This is a follow-up to #56190 (which added `SessionQueryTest` and the
`SparkSessionBinder` /
`SparkSessionBinderBase` traits).
This PR makes the classic `SparkSession` used by the test-session lifecycle
hooks in
`SparkSessionBinderBase` an overridable seam, so a suite can bind its
session per test instead of
only through the single shared `beforeAll` session.
`SparkSessionBinderBase` exposes the bound session through the abstract
`spark` (typed as the
session-agnostic `SparkSession`, so the connect binder can narrow it to a
connect session). Its
per-test `afterEach` hook, however, needs the classic session
(`sharedState.cacheManager`), and
today reads the trait's `private var _spark` directly. That is fine for the
common case where the
session is created in `beforeAll`, but it hard-codes the assumption that
the shared field is the
session the hooks should act on.
Changes:
- Add `protected def classicSpark: classic.SparkSession = _spark` to
`SparkSessionBinderBase` and
route the per-test hook through it (`afterEach`'s
`cacheManager.clearCache()`).
- `connect.SparkSessionBinder` already computed the same value as a
`private def classicSpark`;
change it to `override protected def classicSpark` of the promoted
accessor (no behavior change).
- Add `PerTestSessionBinderSuite`, a small example suite that runs each
test in a fresh instance
(`OneInstancePerTest`) bound to a per-test cloned session, by overriding
`classicSpark`. This both
demonstrates the new flexibility and guards the seam.
### Why are the changes needed?
The current binder can only bind the session created in `beforeAll`. A
suite that wants an isolated
session per test -- e.g. one mixing in `OneInstancePerTest`, where
ScalaTest runs each test in a
fresh suite instance that executes only `beforeEach`/`afterEach` and never
`beforeAll` -- has no way
to tell the inherited hooks which session to operate on; those hooks read
the private shared field,
which is `null` in the per-test instance. Making `classicSpark` overridable
removes that limitation
while leaving the default (shared-session) behavior unchanged.
### Does this PR introduce _any_ user-facing change?
No. This is test-only; the default behavior of `SparkSessionBinderBase` and
the connect binder is
unchanged.
### How was this patch tested?
New `PerTestSessionBinderSuite` exercises the per-test binding path and
passes. Existing binder-based
suites compile and run unchanged (the connect binder change is a `private
def` -> `override
protected def` with the same body).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
Closes #56963 from cloud-fan/cloud-fan/session-binder-classic-accessor.
Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit dad5db794b30de05da1821e1ea280cede15dd1f4)
Signed-off-by: Wenchen Fan <[email protected]>
---
.../spark/sql/connect/SparkSessionBinder.scala | 8 +--
.../org/apache/spark/sql/SparkSessionBinder.scala | 13 +++-
.../spark/sql/test/PerTestSessionBinderSuite.scala | 70 ++++++++++++++++++++++
3 files changed, 84 insertions(+), 7 deletions(-)
diff --git
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/SparkSessionBinder.scala
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/SparkSessionBinder.scala
index c43c3019298a..b30bc55f7df1 100644
---
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/SparkSessionBinder.scala
+++
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/SparkSessionBinder.scala
@@ -21,7 +21,6 @@ import java.util.UUID
import org.apache.spark.{SparkEnv, SparkFunSuite}
import org.apache.spark.sql
-import org.apache.spark.sql.classic
import org.apache.spark.sql.connect.client.SparkConnectClient
import org.apache.spark.sql.connect.config.Connect
import org.apache.spark.sql.connect.service.SparkConnectService
@@ -29,8 +28,8 @@ import
org.apache.spark.sql.connect.service.SparkConnectService
/**
* Provides a [[SparkSession connect.SparkSession]] backed by an in-process
gRPC server. Extends
* [[sql.SparkSessionBinder sql.SparkSessionBinder]] (which creates a
- * [[classic.SparkSession classic.SparkSession]] and SparkContext), then
layers a Connect client
- * session on top by starting the gRPC service in-process.
+ * [[org.apache.spark.sql.classic.SparkSession classic.SparkSession]] and
SparkContext), then
+ * layers a Connect client session on top by starting the gRPC service
in-process.
*/
trait SparkSessionBinder extends sql.SparkSessionBinder { self: SparkFunSuite
=>
@@ -38,9 +37,6 @@ trait SparkSessionBinder extends sql.SparkSessionBinder {
self: SparkFunSuite =>
protected override def spark: SparkSession = _connectSpark
- /** The underlying classic session used by the in-process server. */
- private def classicSpark: classic.SparkSession =
super.spark.asInstanceOf[classic.SparkSession]
-
override protected def beforeAll(): Unit = {
super.beforeAll()
// Other suites using mocks leave a mess in the global executionManager,
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionBinder.scala
b/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionBinder.scala
index 017e35ff4ce4..365901350cad 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionBinder.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/SparkSessionBinder.scala
@@ -110,6 +110,17 @@ trait SparkSessionBinderBase
protected override def spark: SparkSession = _spark
+ /**
+ * The classic [[SparkSession]] this binder is bound to. The per-test
lifecycle hooks operate on
+ * it (e.g. `DebugFilesystem`, cache clearing) via its classic-only APIs,
which the abstract
+ * [[spark]] does not expose (the connect binder narrows `spark` to a
connect session). By default
+ * this is the session created in [[beforeAll]]. Suites that bind their
session outside
+ * `beforeAll` -- e.g. a fresh per-test instance under `OneInstancePerTest`,
where `beforeAll` /
+ * [[initializeSession]] never run and `_spark` stays `null` -- override
this to return their own
+ * session so the inherited hooks act on the live session.
+ */
+ protected def classicSpark: classic.SparkSession = _spark
+
/**
* The [[SQLContext]] to use for all tests in this suite.
*/
@@ -178,7 +189,7 @@ trait SparkSessionBinderBase
protected override def afterEach(): Unit = {
super.afterEach()
// Clear all persistent datasets after each test
- _spark.sharedState.cacheManager.clearCache()
+ classicSpark.sharedState.cacheManager.clearCache()
// files can be closed from other threads, so wait a bit
// normally this doesn't take more than 1s
eventually(timeout(10.seconds), interval(2.seconds)) {
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/test/PerTestSessionBinderSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/test/PerTestSessionBinderSuite.scala
new file mode 100644
index 000000000000..cd070a912cc3
--- /dev/null
+++
b/sql/core/src/test/scala/org/apache/spark/sql/test/PerTestSessionBinderSuite.scala
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.test
+
+import org.scalatest.OneInstancePerTest
+
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.classic.SparkSession
+
+/**
+ * Demonstrates binding a per-test [[SparkSession]] instead of the single
shared session, by
+ * overriding [[SparkSessionBinderBase.classicSpark]].
+ *
+ * With `OneInstancePerTest`, ScalaTest runs each test in a fresh instance of
the suite that
+ * executes only `beforeEach` -- never `beforeAll` / `initializeSession`. The
shared session bound
+ * by the base binder therefore stays `null` in that per-test instance, so the
inherited
+ * `beforeEach` / `afterEach` hooks cannot reach it through the private field.
Overriding
+ * `classicSpark` to return a per-test session (a clone of the shared one, set
up here in
+ * `beforeEach` before `super.beforeEach()` runs) lets the inherited hooks
operate on the live
+ * session. This is the seam a parallelized / isolated-session test runner
builds on.
+ */
+class PerTestSessionBinderSuite
+ extends SharedSparkSession
+ with OneInstancePerTest {
+
+ // The session bound for the currently running test. Set in beforeEach on
the per-test instance.
+ private var perTestSpark: SparkSession = _
+
+ // Route the inherited before/after hooks (and the suite's `spark`) at the
per-test session.
+ override protected def classicSpark: SparkSession = perTestSpark
+ override protected def spark: SparkSession = perTestSpark
+
+ override protected def beforeEach(): Unit = {
+ // Clone the shared session so each test gets an isolated one; must be set
before the base
+ // beforeEach runs, since that hook now reads `classicSpark`.
+ perTestSpark = SparkSession.getActiveSession
+ .orElse(SparkSession.getDefaultSession)
+ .getOrElse(createSparkSession)
+ .cloneSession()
+ SparkSession.setActiveSession(perTestSpark)
+ super.beforeEach()
+ }
+
+ override protected def afterEach(): Unit = {
+ super.afterEach()
+ SparkSession.clearActiveSession()
+ }
+
+ test("per-test session is bound so the inherited lifecycle hooks operate on
a live session") {
+ // A successful run proves the inherited beforeEach/afterEach reached a
live session through
+ // the overridden `classicSpark` rather than NPEing on the null shared
field.
+ assert(perTestSpark != null)
+ checkAnswer(perTestSpark.sql("SELECT 1"), Row(1))
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
