philo-he commented on code in PR #12506:
URL: https://github.com/apache/gluten/pull/12506#discussion_r3583400148
##########
gluten-ut/spark40/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala:
##########
@@ -734,10 +734,26 @@ class VeloxTestSettings extends BackendTestSettings {
enableSuite[GlutenLogicalPlanTagInSparkPlanSuite]
enableSuite[GlutenOptimizeMetadataOnlyQuerySuite]
enableSuite[GlutenPersistedViewTestSuite]
- // TODO: 4.x enableSuite[GlutenPlannerSuite] // 1 failure
- // TODO: 4.x enableSuite[GlutenProjectedOrderingAndPartitioningSuite] // 6
failures
+ // GlutenPlannerSuite is not enabled: it validates Spark planner
implementation details.
+ // GlutenProjectedOrderingAndPartitioningSuite is not enabled: it validates
Spark planner
Review Comment:
@zml1206, Could we add a `disableSuite[X]("reason")` method in
`BackendTestSettings.scala` to track intentionally disabled suites? This would
be more explicit and easier to track than the current comment-based approach.
Today we can get a similar effect with enableSuite[X].disable("reason"), but
that reads oddly: we enable a suite and then immediately disable it. A
dedicated disableSuite would state the intent directly.
```
private val disabledSuites: java.util.Map[String, String] = new
util.HashMap() // name -> reason
protected def disableSuite[T: ClassTag](reason: String): Unit =
disableSuite(implicitly[ClassTag[T]].runtimeClass.getCanonicalName, reason)
protected def disableSuite(suiteName: String, reason: String): Unit = {
require(reason.nonEmpty, "Disable reason must not be empty")
if (enabledSuites.containsKey(suiteName)) {
throw new IllegalArgumentException("Suite is already enabled: " +
suiteName)
}
if (disabledSuites.containsKey(suiteName)) {
throw new IllegalArgumentException("Duplicated disabled suite: " +
suiteName)
}
disabledSuites.put(suiteName, reason)
}
```
--
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]