jackylee-ch commented on code in PR #9274:
URL: https://github.com/apache/incubator-gluten/pull/9274#discussion_r2036358869
##########
gluten-hudi/src/test/scala/org/apache/gluten/execution/HudiSuite.scala:
##########
@@ -85,7 +85,7 @@ abstract class HudiSuite extends WholeStageTransformerSuite {
}
// FIXME: flaky leaked file systems issue
- ignore("hudi: mor", Some("3.2")) {
+ ignoreWithSpecifiedSparkVersion("hudi: mor", Some("3.2")) {
Review Comment:
this can be changed to ignore
##########
gluten-iceberg/src/test/scala/org/apache/gluten/execution/IcebergSuite.scala:
##########
@@ -284,7 +282,8 @@ abstract class IcebergSuite extends
WholeStageTransformerSuite {
testWithSpecifiedSparkVersion(
Review Comment:
nit: this can be changed to `testWithMinSparkVersion`
##########
gluten-iceberg/src/test/scala/org/apache/gluten/execution/IcebergSuite.scala:
##########
@@ -205,9 +205,7 @@ abstract class IcebergSuite extends
WholeStageTransformerSuite {
}
}
- testWithSpecifiedSparkVersion(
- "iceberg bucketed join partition value not exists",
- Array("3.4", "3.5")) {
+ testWithSpecifiedSparkVersion("iceberg bucketed join partition value not
exists", "3.4", "3.5") {
Review Comment:
ditto
##########
gluten-substrait/src/test/scala/org/apache/spark/sql/GlutenQueryTest.scala:
##########
@@ -77,31 +91,49 @@ abstract class GlutenQueryTest extends PlanTest {
shouldRun
}
- def ignore(
+ /** Ignore the test if the current spark version is between the minVersion
and maxVersion */
+ def ignoreWithSpecifiedSparkVersion(
testName: String,
minSparkVersion: Option[String] = None,
maxSparkVersion: Option[String] = None)(testFun: => Any): Unit = {
- if (shouldRun(minSparkVersion, maxSparkVersion)) {
+ if (matchSparkVersion(minSparkVersion, maxSparkVersion)) {
ignore(testName) {
testFun
}
}
}
- def testWithSpecifiedSparkVersion(
- testName: String,
- minSparkVersion: Option[String] = None,
- maxSparkVersion: Option[String] = None)(testFun: => Any): Unit = {
- if (shouldRun(minSparkVersion, maxSparkVersion)) {
+ /** Run the test if the current spark version is between the minVersion and
maxVersion */
+ def testWithRangeSparkVersion(testName: String, minSparkVersion: String,
maxSparkVersion: String)(
+ testFun: => Any): Unit = {
+ if (matchSparkVersion(Some(minSparkVersion), Some(maxSparkVersion))) {
test(testName) {
testFun
}
}
}
- def testWithSpecifiedSparkVersion(testName: String, versions: Array[String])(
- testFun: => Any): Unit = {
- if (versions.exists(v => shouldRun(Some(v), Some(v)))) {
+ /** Run the test if the current spark version less than the maxVersion */
+ def testAtMaxSparkVersion(testName: String, maxVersion: String)(testFun: =>
Any): Unit = {
+ if (matchSparkVersion(maxSparkVersion = Some(maxVersion))) {
+ test(testName) {
+ testFun
+ }
+ }
+ }
+
+ /** Run the test if the current spark version greater than the minVersion */
+ def testAtLeastSparkVersion(testName: String, minVersion: String)(testFun:
=> Any): Unit = {
Review Comment:
nit: `testWithMinSparkVersion` and `testWithMaxSparkVersion` sounds better
to me.
##########
backends-velox/src/test/scala/org/apache/spark/sql/execution/VeloxParquetReadSuite.scala:
##########
@@ -32,7 +32,7 @@ class VeloxParquetReadSuite extends
VeloxWholeStageTransformerSuite {
.set(VeloxConfig.LOAD_QUANTUM.key, "256m")
}
- testWithSpecifiedSparkVersion("read example parquet files", Some("3.5"),
Some("3.5")) {
+ testWithSpecifiedSparkVersion("read example parquet files", "3.5") {
Review Comment:
Also, for these cases, `testWithMinSparkVersion` sounds better to me.
--
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]
