srowen commented on a change in pull request #23258: 
[SPARK-23375][SQL][FOLLOWUP][TEST] Test Sort metrics while Sort is missing
URL: https://github.com/apache/spark/pull/23258#discussion_r244533938
 
 

 ##########
 File path: 
sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsTestUtils.scala
 ##########
 @@ -40,6 +41,26 @@ trait SQLMetricsTestUtils extends SQLTestUtils {
 
   protected def statusStore: SQLAppStatusStore = spark.sharedState.statusStore
 
+  protected val bytes = "([0-9]+(\\.[0-9]+)?) (EiB|PiB|TiB|GiB|MiB|KiB|B)"
+
+  protected val duration = "([0-9]+(\\.[0-9]+)?) (ms|s|m|h)"
+
+  // "\n96.2 MiB (32.1 MiB, 32.1 MiB, 32.1 MiB)"
+  protected val sizeMetricPattern = Pattern.compile(s"\\n$bytes \\($bytes, 
$bytes, $bytes\\)")
+
+  // "\n2.0 ms (1.0 ms, 1.0 ms, 1.0 ms)"
+  protected val timingMetricPattern =
+    Pattern.compile(s"\\n$duration \\($duration, $duration, $duration\\)")
+
+  /** Generate a function to check the specified pattern.
+   *
+   * @param pattern a pattern
+   * @return a function to check the specified pattern
+   */
+  protected def checkPattern(pattern: Pattern): (Any => Boolean) = {
 
 Review comment:
   Is this method really needed? the only place it's used is the very specific 
method for testing metrics, and that always provides a regex. Just provide a 
map to regexes that you check against, rather than whole predicates?
   
   Or, consider _not_ compiling regexes above and keeping them as string 
patterns. Then, the predicate you pass is just something like 
`sizeMetricPattern.matches(_)`. It means compiling the regex on every check, 
but for this test context, that's no big deal.
   
   That would help limit the complexity of all this

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to