houserjohn commented on code in PR #14238: URL: https://github.com/apache/lucene/pull/14238#discussion_r1956936618
########## lucene/facet/src/test/org/apache/lucene/facet/range/TestDynamicRangeUtil.java: ########## @@ -76,13 +77,248 @@ public void testComputeDynamicNumericRangesWithOneLargeWeight() { long[] values = new long[] {45, 32, 52, 14, 455, 342, 53}; long[] weights = new long[] {143, 23, 1, 52343, 53, 12, 2534}; - // value 14 has its own bin since the weight is large, and the rest of values fall the other bin - expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343, 14L, 14L, 14D)); + // value 14 has its own bin since the weight is large, and the rest of the values fall in the + // other bin + expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343L, 14L, 14L, 14D)); expectedRangeInfoList.add( - new DynamicRangeUtil.DynamicRangeInfo(6, 2766, 32L, 455L, 163.16666666666666D)); + new DynamicRangeUtil.DynamicRangeInfo(6, 2766L, 32L, 455L, 163.16666666666666D)); assertDynamicNumericRangeResults(values, weights, 4, 55109, expectedRangeInfoList); } + public void testComputeDynamicNumericRangesWithLargeTopN() { + List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new ArrayList<>(); + long[] values = new long[] {487, 439, 794, 277}; + long[] weights = new long[] {59, 508, 736, 560}; + + // More requested ranges (TopN) than values should return ranges with weights larger than the + // average weight - excluding the last range + expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 560L, 277L, 277L, 277D)); + expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 508L, 439L, 439L, 439D)); + expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 487L, 794L, 640.5D)); + assertDynamicNumericRangeResults(values, weights, 42, 1863L, expectedRangeInfoList); + } + + public void testComputeDynamicNumericRangesWithSingleTopN() { + List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new ArrayList<>(); + long[] values = new long[] {487, 439, 794, 277}; + long[] weights = new long[] {59, 508, 736, 560}; + + // A single requested range (TopN) should return a single range regardless of the weights + // provided + expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(4, 1863L, 277L, 794L, 499.25D)); + assertDynamicNumericRangeResults(values, weights, 1, 1863L, expectedRangeInfoList); + } + + public void testComputeDynamicNumericRangesWithTwoTopN() { Review Comment: Originally, `testComputeDynamicNumericRangesWithLargeTopN` revealed a bug (mentioned above), so it was added to prevent us from re-encountering it again. This test didn't have a bug that directly inspired it, but we did not have a previous test that checks small topN. For completeness, I thought it made sense to have one for `N=1` and `N=2`. -- 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