Fix the parent join aggregator test case

The test was putting parent and child documents into different segments which is unrealistic and was causing errors. Closes #60980
elastic · Aug 11, 2020 · d331afb · d331afb
1 parent 766177f
commit d331afb
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 14 deletions.
diff --git a/...in/src/test/java/org/elasticsearch/join/aggregations/ChildrenToParentAggregatorTests.java b/...in/src/test/java/org/elasticsearch/join/aggregations/ChildrenToParentAggregatorTests.java
@@ -32,20 +32,19 @@
 import org.apache.lucene.search.TermInSetQuery;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.BytesRef;
-import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.mapper.ContentPath;
-import org.elasticsearch.index.mapper.MappingLookup;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.IdFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.mapper.MappingLookup;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
 import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.shard.ShardId;
@@ -75,7 +74,6 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/60980")
 public class ChildrenToParentAggregatorTests extends AggregatorTestCase {
 
     private static final String CHILD_TYPE = "child_type";
@@ -121,9 +119,11 @@ public void testParentChild() throws IOException {
                 expectedTotalParents++;
                 expectedMinValue = Math.min(expectedMinValue, expectedValues.v2());
             }
-            assertEquals("Having " + parent.getDocCount() + " docs and aggregation results: " +
-                    parent.getAggregations().asMap(),
-                expectedTotalParents, parent.getDocCount());
+            assertEquals(
+                "Having " + parent.getDocCount() + " docs and aggregation results: " + parent,
+                expectedTotalParents,
+                parent.getDocCount()
+            );
             assertEquals(expectedMinValue, ((InternalMin) parent.getAggregations().get("in_parent")).getValue(), Double.MIN_VALUE);
             assertTrue(JoinAggregationInspectionHelper.hasValue(parent));
         });
@@ -235,21 +235,19 @@ private static Map<String, Tuple<Integer, Integer>> setupIndex(RandomIndexWriter
         Map<String, Tuple<Integer, Integer>> expectedValues = new HashMap<>();
         int numParents = randomIntBetween(1, 10);
         for (int i = 0; i < numParents; i++) {
+            List<List<Field>> documents = new ArrayList<>();
             String parent = "parent" + i;
             int randomValue = randomIntBetween(0, 100);
-            List<Field> parentDocument = createParentDocument(parent, randomValue);
-            /*long parentDocId =*/ iw.addDocument(parentDocument);
-            //System.out.println("Parent: " + parent + ": " + randomValue + ", id: " + parentDocId);
+            documents.add(createParentDocument(parent, randomValue));
             int numChildren = randomIntBetween(1, 10);
             int minValue = Integer.MAX_VALUE;
             for (int c = 0; c < numChildren; c++) {
                 minValue = Math.min(minValue, randomValue);
                 int randomSubValue = randomIntBetween(0, 100);
-                List<Field> childDocument = createChildDocument("child" + c + "_" + parent, parent, randomSubValue);
-                /*long childDocId =*/ iw.addDocument(childDocument);
-                //System.out.println("Child: " + "child" + c + "_" + parent + ": " + randomSubValue + ", id: " + childDocId);
+                documents.add(createChildDocument("child" + c + "_" + parent, parent, randomSubValue));
             }
             expectedValues.put(parent, new Tuple<>(numChildren, minValue));
+            iw.addDocuments(documents);
         }
         return expectedValues;
     }

diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java
@@ -419,9 +419,12 @@ protected <A extends InternalAggregation, C extends Aggregator> A searchAndReduc
     }
 
     /**
-     * Divides the provided {@link IndexSearcher} in sub-searcher, one for each segment,
-     * builds an aggregator for each sub-searcher filtered by the provided {@link Query} and
+     * Collects all documents that match the provided query {@link Query} and
      * returns the reduced {@link InternalAggregation}.
+     * <p>
+     * Half the time it aggregates each leaf individually and reduces all
+     * results together. The other half the time it aggregates across the entire
+     * index at once and runs a final reduction on the single resulting agg.
      */
     protected <A extends InternalAggregation, C extends Aggregator> A searchAndReduce(IndexSettings indexSettings,
                                                                                       IndexSearcher searcher,