felipepessoto commented on code in PR #13131:
URL: https://github.com/apache/gluten/pull/13131#discussion_r4405575202
##########
.github/workflows/util/delta-spark-ut/README.md:
##########
@@ -293,3 +328,6 @@ source
<gluten>/.github/workflows/util/delta-spark-ut/java-test-args.sh
`run-delta-tests.sh` sources the same file, so CI and local runs use identical
flags.
+
+Existing `JAVA_TOOL_OPTIONS` are preserved after the shared defaults, so
+caller-supplied `-D` properties take precedence.
Review Comment:
The first sentence already states that the entire existing
`JAVA_TOOL_OPTIONS` is preserved after the shared defaults. The narrower
precedence statement about `-D` properties is intentional: JVM options do not
all follow last-value-wins semantics—for example, `--add-opens` can accumulate.
I'll keep this wording rather than imply that all caller-supplied flags replace
earlier options.
##########
.github/workflows/util/delta-spark-ut/java-test-args.sh:
##########
@@ -35,11 +35,22 @@
# the sbt LAUNCHER JVM) so the flags reach BOTH the launcher and the forked
test JVM
# -- the forked child inherits the parent env.
#
+# SparkConf also reads spark.* system properties as defaults. Keep all Gluten
+# test defaults here instead of replacing DeltaSQLCommandTest, so suites using
+# plain SharedSparkSession, TestHive or their own SparkSession builders inherit
+# the same settings. Those contexts must initialize Gluten too: Spark retains
+# its cache serializer across contexts in the same JVM, but the native
+# serializer needs the executor plugin to initialize TaskResources in each
task.
+#
+# Keep Delta's extensions and catalog suite-specific: some tests deliberately
+# omit them to check Delta's configuration errors.
+#
# NOTE: no -Xmx here on purpose. JAVA_TOOL_OPTIONS is processed BEFORE the JVM
# command line, so Delta's own -Xmx (build.sbt) would win; run-delta-tests.sh
bumps
# the forked-test-JVM heap via `set spark / Test / javaOptions ++= ...`
instead.
-export JAVA_TOOL_OPTIONS="${JAVA_TOOL_OPTIONS:+${JAVA_TOOL_OPTIONS} }\
+# Keep caller-provided -D properties last so they override these defaults.
Review Comment:
Appending the full existing `JAVA_TOOL_OPTIONS` is intentional. The comment
calls out the `-D` precedence we rely on; it doesn't say that only those
properties are preserved or that other options are filtered. I'll keep the
current wording and implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
