[GitHub] [incubator-pinot] Ruoyingw commented on a change in pull request #5861: [TE] merge time series snapshot when merging anomalies

[GitBox <gi...@apache.org>]

Ruoyingw commented on a change in pull request #5861:
URL: https://github.com/apache/incubator-pinot/pull/5861#discussion_r470793110



##########
File path: thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/util/ThirdEyeUtilsTest.java
##########
@@ -176,21 +179,51 @@ public void testExceptionToStringLimited() {
   }
 
   @Test
-  public void testMergeAnomalyProperties() {
+  public void testMergeAnomalyProperties() throws Exception {
+    final long now = System.currentTimeMillis();
+    final long bucketSize = 300_000;
+    final double parentVal = 1.0, childVal = 2.0;
+
     Map<String, String> parentProperties = new HashMap<>();
     parentProperties.put("p1", "value1");
     parentProperties.put("p2", "value2");
     parentProperties.put("detectorComponentName", "rule1");
+    parentProperties.put("anomalyTimelinesView",
+        generateAnomalyTimelineView(now, bucketSize, 10, parentVal).toJsonString());
+
 
     Map<String, String> childProperties = new HashMap<>();
     childProperties.put("p1", "value3");
     childProperties.put("c1", "value4");
     childProperties.put("detectorComponentName", "rule2");
+    childProperties.put("anomalyTimelinesView",
+        generateAnomalyTimelineView(now + bucketSize, bucketSize, 10,  childVal).toJsonString());

Review comment:
       Can we add another unit test case where the child anomaly has a gap with parent? say 
   
   `childProperties.put("anomalyTimelinesView", generateAnomalyTimelineView(now + bucketSize * 20, bucketSize, 10,  childVal).toJsonString());`

##########
File path: thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/util/ThirdEyeUtilsTest.java
##########
@@ -176,21 +179,51 @@ public void testExceptionToStringLimited() {
   }
 
   @Test
-  public void testMergeAnomalyProperties() {
+  public void testMergeAnomalyProperties() throws Exception {
+    final long now = System.currentTimeMillis();
+    final long bucketSize = 300_000;
+    final double parentVal = 1.0, childVal = 2.0;
+
     Map<String, String> parentProperties = new HashMap<>();
     parentProperties.put("p1", "value1");
     parentProperties.put("p2", "value2");
     parentProperties.put("detectorComponentName", "rule1");
+    parentProperties.put("anomalyTimelinesView",
+        generateAnomalyTimelineView(now, bucketSize, 10, parentVal).toJsonString());
+
 
     Map<String, String> childProperties = new HashMap<>();
     childProperties.put("p1", "value3");
     childProperties.put("c1", "value4");
     childProperties.put("detectorComponentName", "rule2");
+    childProperties.put("anomalyTimelinesView",
+        generateAnomalyTimelineView(now + bucketSize, bucketSize, 10,  childVal).toJsonString());
 
     ThirdEyeUtils.mergeAnomalyProperties(parentProperties, childProperties);
     Assert.assertEquals(parentProperties.get("p1"), "value1");
     Assert.assertEquals(parentProperties.get("p2"), "value2");
     Assert.assertEquals(parentProperties.get("c1"), "value4");
     Assert.assertEquals(parentProperties.get("detectorComponentName"), "rule1,rule2");
+    AnomalyTimelinesView merged = AnomalyTimelinesView.fromJsonString(parentProperties.get("anomalyTimelinesView"));
+    Assert.assertEquals(merged.getTimeBuckets().size(), 11);
+    Assert.assertTrue(merged.getCurrentValues().get(0) - parentVal < 0.00001);
+    Assert.assertTrue(merged.getCurrentValues().get(1) - parentVal < 0.00001);
+    Assert.assertTrue(merged.getCurrentValues().get(10) - childVal < 0.00001);

Review comment:
       Probably better to compare with absolute difference: `abs(actual - expected) < 0.00001`. Or better:
   we can use `Assert.assertEquals(actual, expected, delta)`




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org