gsmiller commented on a change in pull request #131:
URL: https://github.com/apache/lucene/pull/131#discussion_r630606547
##########
File path:
lucene/facet/src/test/org/apache/lucene/facet/TestLongValueFacetCounts.java
##########
@@ -429,15 +429,15 @@ public void testRandomMultiValued() throws Exception {
int expectedChildCount = 0;
int expectedTotalCount = 0;
for (int i = 0; i < valueCount; i++) {
- if (values[i] != null) {
+ if (values[i] != null && values[i].length > 0) {
Review comment:
Thanks @gautamworah96 . I'm not sure I fully understand the suggestion
to add a duplicate long value to test the change? The current bug is that,
whenever a document has multiple values for a particular field (they don't need
to be the _same_ value), we'll count that document multiple times (once for
each value it contains) in FacetResult#value. So I think it should be
sufficient to just make sure documents contain multiple values to trip the bug,
which is happening in Line 396 in this test case. The change I made to the test
case reflects the proper logic for populating FacetResult#value, and does in
fact fail if my change is not present.
Does this make sense? Maybe I'm missing your point. If so, I apologize!
--
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: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
