Repository navigation
[CORE] Limit SparkSessionExtension suite to Gluten test - #12475
Conversation
There was a problem hiding this comment.
Pull request overview
This PR re-enables GlutenSparkSessionExtensionSuite for Spark 4.0/4.1 Velox unit tests by removing reliance on GlutenTestsTrait (which initializes a shared SparkSession in beforeAll) and switching to GlutenTestsCommonTrait to preserve Gluten test filtering while allowing Spark’s upstream SparkSessionExtensionSuite to create per-test sessions with custom extensions.
Changes:
- Update Spark 4.0 and 4.1
GlutenSparkSessionExtensionSuiteto mix inGlutenTestsCommonTraitinstead ofGlutenTestsTrait, avoiding shared-session side effects. - Enable
GlutenSparkSessionExtensionSuitein Spark 4.0 and 4.1 Velox test settings.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| gluten-ut/spark41/src/test/scala/org/apache/spark/sql/GlutenSparkSessionExtensionSuite.scala | Switches from GlutenTestsTrait to GlutenTestsCommonTrait to prevent shared-session initialization from interfering with per-test extensions. |
| gluten-ut/spark41/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Enables GlutenSparkSessionExtensionSuite in Spark 4.1 Velox test matrix. |
| gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenSparkSessionExtensionSuite.scala | Same trait switch for Spark 4.0 to avoid shared-session behavior. |
| gluten-ut/spark40/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Enables GlutenSparkSessionExtensionSuite in Spark 4.0 Velox test matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@zml1206, thanks for the fix. One comment, which might be slightly beyond the scope of this PR. Please check if it makes sense. I noticed this suite runs all the parent SparkSessionExtensionSuite tests without the Gluten plugin or any Gluten-specific configuration. Those are Spark's own tests and can be covered on the Spark side, so there's no need to run them on Gluten. Could we use includeGlutenTest("customColumnarOp") to enable only this Gluten test, where the Gluten plugin and related config are actually applied to the Spark session? This test was introduced by #4165 and isn't intended to cover Spark's own tests the way other suites do. Alternatively, if the test doesn't really depend on SparkSessionExtensionSuite, could we move it to a new suite under gluten-ut/test? |
|
Run Gluten Clickhouse CI on x86 |
Thanks for the suggestion. It makes sense to avoid running the parent SparkSessionExtensionSuite tests here since they are Spark-side coverage and do not require Gluten-specific configuration. I'll update the suite settings to enable only the Gluten-specific test with includeGlutenTest("customColumnarOp"). I prefer this over moving the test for now because the test still reuses SparkSessionExtensionSuite helpers, and the scoped include keeps the PR minimal. |
| enableSuite[GlutenSparkSessionExtensionSuite] | ||
| .includeGlutenTest("customColumnarOp") |
zml1206
changed the title
Enable GlutenSparkSessionExtensionSuite for Spark 4.0/4.1 Velox UTs.
The suite should not mix in GlutenTestsTrait, because that trait initializes a shared Gluten Spark session in beforeAll. Spark’s upstream SparkSessionExtensionSuite creates sessions with custom extensions per test, and reusing an existing session may cause injected extensions such as analyzer rules to be ignored.
This patch changes Spark 4.0/4.1 GlutenSparkSessionExtensionSuite to use GlutenTestsCommonTrait, matching the suite’s need for Gluten test filtering without creating a shared session, then enables the suite in Velox test settings.