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]

Reply via email to