Yuti-G commented on code in PR #1013:
URL: https://github.com/apache/lucene/pull/1013#discussion_r948407058


##########
lucene/facet/src/test/org/apache/lucene/facet/range/TestRangeFacetCounts.java:
##########
@@ -100,12 +100,21 @@ public void testBasicLong() throws Exception {
             new LongRange("90 or above", 90L, true, 100L, false),
             new LongRange("over 1000", 1000L, false, Long.MAX_VALUE, true));
 
-    FacetResult result = facets.getAllChildren("field");
-    assertEquals(
-        "dim=field path=[] value=22 childCount=5\n  less than 10 (10)\n  less 
than or equal to 10 (11)\n  over 90 (9)\n  90 or above (10)\n  over 1000 (1)\n",
-        result.toString());
+    assertFacetResult(

Review Comment:
   Sorry, I am confused. Our javadoc for Facets#getAllChildren explicitly calls 
out that callers should make _**NO**_ assumptions about child ordering. Isn't 
the purpose of addressing the previous tests by ignoring child order like you 
described in the LUCENE-10644 Jira issue? 
   
   I know it's been a while, but please refer to our PR comments and confirm 
whether we misunderstand something here. Thanks! 
   
   > Thanks @Yuti-G! This approach looks good to me. Is your plan to iterate on 
this PR to stop enforcing the ordering checks in all the tests?



-- 
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: issues-unsubscr...@lucene.apache.org

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


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

Reply via email to