siddharthteotia commented on a change in pull request #6625:
URL: https://github.com/apache/incubator-pinot/pull/6625#discussion_r591786053



##########
File path: 
pinot-controller/src/test/java/org/apache/pinot/controller/recommender/realtime/provisioning/MemoryEstimatorTest.java
##########
@@ -41,20 +43,20 @@
   public void testSegmentGenerator() throws Exception {
     runTest("memory_estimation/schema-with-metadata.json", metadata -> {
       assertEquals(extract(metadata, "segment.total.docs = (\\d+)"), "100000");
-      assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), 
"100");
-      assertEquals(extract(metadata, "column.colIntMV.cardinality = (\\d+)"), 
"150");
-      assertEquals(extract(metadata, "column.colFloat.cardinality = (\\d+)"), 
"200");
-      assertEquals(extract(metadata, "column.colFloatMV.cardinality = 
(\\d+)"), "250");
-      assertEquals(extract(metadata, "column.colString.cardinality = (\\d+)"), 
"300");
-      assertEquals(extract(metadata, "column.colStringMV.cardinality = 
(\\d+)"), "350");
-      assertEquals(extract(metadata, "column.colBytes.cardinality = (\\d+)"), 
"400");
-      assertEquals(extract(metadata, "column.colLong.cardinality = (\\d+)"), 
"500");
-      assertEquals(extract(metadata, "column.colLongMV.cardinality = (\\d+)"), 
"550");
-      assertEquals(extract(metadata, "column.colDouble.cardinality = (\\d+)"), 
"600");
-      assertEquals(extract(metadata, "column.colDoubleMV.cardinality = 
(\\d+)"), "650");
-      assertEquals(extract(metadata, "column.colDoubleMetric.cardinality = 
(\\d+)"), "700");
-      assertEquals(extract(metadata, "column.colFloatMetric.cardinality = 
(\\d+)"), "800");
-      assertEquals(extract(metadata, "column.colTime.cardinality = (\\d+)"), 
"900");
+      assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), 
"10");

Review comment:
       As discussed offline, this is a good way to make test deterministic. 
Smaller cardinality might ensure that random generator will generate as many 
unique values as we want but it is still not deterministic. Secondly, smaller 
cardinality is not very meaningful for the test. So let's fix the generation 
code to ensure it generates as many unique values as we want in a deterministic 
style.

##########
File path: 
pinot-controller/src/test/java/org/apache/pinot/controller/recommender/realtime/provisioning/MemoryEstimatorTest.java
##########
@@ -41,20 +43,20 @@
   public void testSegmentGenerator() throws Exception {
     runTest("memory_estimation/schema-with-metadata.json", metadata -> {
       assertEquals(extract(metadata, "segment.total.docs = (\\d+)"), "100000");
-      assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), 
"100");
-      assertEquals(extract(metadata, "column.colIntMV.cardinality = (\\d+)"), 
"150");
-      assertEquals(extract(metadata, "column.colFloat.cardinality = (\\d+)"), 
"200");
-      assertEquals(extract(metadata, "column.colFloatMV.cardinality = 
(\\d+)"), "250");
-      assertEquals(extract(metadata, "column.colString.cardinality = (\\d+)"), 
"300");
-      assertEquals(extract(metadata, "column.colStringMV.cardinality = 
(\\d+)"), "350");
-      assertEquals(extract(metadata, "column.colBytes.cardinality = (\\d+)"), 
"400");
-      assertEquals(extract(metadata, "column.colLong.cardinality = (\\d+)"), 
"500");
-      assertEquals(extract(metadata, "column.colLongMV.cardinality = (\\d+)"), 
"550");
-      assertEquals(extract(metadata, "column.colDouble.cardinality = (\\d+)"), 
"600");
-      assertEquals(extract(metadata, "column.colDoubleMV.cardinality = 
(\\d+)"), "650");
-      assertEquals(extract(metadata, "column.colDoubleMetric.cardinality = 
(\\d+)"), "700");
-      assertEquals(extract(metadata, "column.colFloatMetric.cardinality = 
(\\d+)"), "800");
-      assertEquals(extract(metadata, "column.colTime.cardinality = (\\d+)"), 
"900");
+      assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), 
"10");

Review comment:
       As discussed offline, this is not a good way to make test deterministic. 
Smaller cardinality might ensure that random generator will generate as many 
unique values as we want but it is still not deterministic. Secondly, smaller 
cardinality is not very meaningful for the test. So let's fix the generation 
code to ensure it generates as many unique values as we want in a deterministic 
style.




----------------------------------------------------------------
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: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to