HeartSaVioR commented on a change in pull request #31435:
URL: https://github.com/apache/spark/pull/31435#discussion_r568565607



##########
File path: 
sql/core/src/test/scala/org/apache/spark/sql/execution/DataSourceScanExecRedactionSuite.scala
##########
@@ -137,9 +137,24 @@ class DataSourceScanExecRedactionSuite extends 
DataSourceScanRedactionTest {
       assert(location.isDefined)
       // The location metadata should at least contain one path
       assert(location.get.contains(paths.head))
-      // If the temp path length is larger than 100, the metadata length 
should not exceed
-      // twice of the length; otherwise, the metadata length should be 
controlled within 200.
-      assert(location.get.length < Math.max(paths.head.length, 100) * 2)
+
+      // The location metadata should have bracket wrapping paths
+      assert(location.get.indexOf('[') > -1)
+      assert(location.get.indexOf(']') > -1)
+
+      // extract paths in location metadata (removing classname, brackets, 
separators)
+      val pathsInLocation = location.get.substring(
+        location.get.indexOf('[') + 1, location.get.indexOf(']')).split(", 
").toSeq
+
+      // If the temp path length is less than (stop appending threshold - 1), 
say, 100 - 1 = 99,
+      // location should include more than one paths. Otherwise location 
should include only one
+      // path.
+      // (Note we apply subtraction with 1 to count start bracket '['.)
+      if (paths.head.length < 99) {
+        assert(pathsInLocation.size >= 2)

Review comment:
       I'm not sure I understand. Could you please elaborate?
   
   We don't truncate the path itself, right? I think it's also something to be 
fixed (I'd rather want to see path being truncated with ellipses (...) instead 
of not adding and leaving it as it is.) but it's more likely bigger fix which 
may worth another fix instead of test fix.
   
   If you meant counting the number of paths or something for edge-case, 
dealing with another UT would be easier, like we could simply add edge-cases 
there in this PR.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to