Pārlūkot izejas kodu

Fix assertion catching in aggregation supported type test (#56466)

At some point, we changed the supported-type test to also catch
assertion errors.  This has the side effect of also catching the
`fail()` call inside the try-catch, which silently smothered some
failures.

This modifies the test to throw at the end of the try-catch
block to prevent from accidentally catching itself.

Catching the AssertionError is convenient because there are other locations
that do throw an assertion in tests (due to hitting an assertion
before the exception is thrown) so I think we should keep it around.

Also includes a variety of fixes to other tests which were failing
but being silently smothered.
Zachary Tong 5 gadi atpakaļ
vecāks
revīzija
792087b316
13 mainītis faili ar 69 papildinājumiem un 16 dzēšanām
  1. 4 1
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTermsAggregatorTests.java
  2. 4 1
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTextAggregatorTests.java
  3. 3 1
      server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java
  4. 3 2
      server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java
  5. 3 2
      server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java
  6. 3 2
      server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java
  7. 4 1
      server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountAggregatorTests.java
  8. 6 2
      test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java
  9. 2 0
      x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java
  10. 2 0
      x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java
  11. 3 0
      x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java
  12. 30 3
      x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregatorTests.java
  13. 2 1
      x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java

+ 4 - 1
server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTermsAggregatorTests.java

@@ -89,7 +89,10 @@ public class SignificantTermsAggregatorTests extends AggregatorTestCase {
     @Override
     protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
         return List.of(CoreValuesSourceType.NUMERIC,
-            CoreValuesSourceType.BYTES);
+            CoreValuesSourceType.BYTES,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE,
+            CoreValuesSourceType.IP);
     }
 
     @Override

+ 4 - 1
server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTextAggregatorTests.java

@@ -81,7 +81,10 @@ public class SignificantTextAggregatorTests extends AggregatorTestCase {
         return List.of(CoreValuesSourceType.NUMERIC,
             CoreValuesSourceType.BYTES,
             CoreValuesSourceType.RANGE,
-            CoreValuesSourceType.GEOPOINT);
+            CoreValuesSourceType.GEOPOINT,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE,
+            CoreValuesSourceType.IP);
     }
 
     @Override

+ 3 - 1
server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java

@@ -703,7 +703,9 @@ public class AvgAggregatorTests extends AggregatorTestCase {
     @Override
     protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
         return List.of(
-            CoreValuesSourceType.NUMERIC
+            CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE
         );
     }
 

+ 3 - 2
server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java

@@ -91,7 +91,6 @@ import java.util.function.Supplier;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singleton;
-import static java.util.Collections.singletonList;
 import static org.elasticsearch.index.query.QueryBuilders.termQuery;
 import static org.hamcrest.Matchers.equalTo;
 
@@ -163,7 +162,9 @@ public class MaxAggregatorTests extends AggregatorTestCase {
 
     @Override
     protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
-        return singletonList(CoreValuesSourceType.NUMERIC);
+        return List.of(CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE);
     }
 
     @Override

+ 3 - 2
server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java

@@ -60,7 +60,6 @@ import java.util.function.Function;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
 import static java.util.Collections.singleton;
-import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
@@ -475,7 +474,9 @@ public class StatsAggregatorTests extends AggregatorTestCase {
 
     @Override
     protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
-        return singletonList(CoreValuesSourceType.NUMERIC);
+        return List.of(CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE);
     }
 
     @Override

+ 3 - 2
server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java

@@ -68,7 +68,6 @@ import java.util.function.Function;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singleton;
-import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static java.util.stream.Collectors.toList;
 import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
@@ -403,7 +402,9 @@ public class SumAggregatorTests extends AggregatorTestCase {
 
     @Override
     protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
-        return singletonList(CoreValuesSourceType.NUMERIC);
+        return List.of(CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE);
     }
 
     @Override

+ 4 - 1
server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountAggregatorTests.java

@@ -89,7 +89,10 @@ public class ValueCountAggregatorTests extends AggregatorTestCase {
             CoreValuesSourceType.NUMERIC,
             CoreValuesSourceType.BYTES,
             CoreValuesSourceType.GEOPOINT,
-            CoreValuesSourceType.RANGE
+            CoreValuesSourceType.RANGE,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE,
+            CoreValuesSourceType.IP
         );
     }
 

+ 6 - 2
test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java

@@ -717,19 +717,23 @@ public abstract class AggregatorTestCase extends ESTestCase {
 
                     ValuesSourceType vst = fieldType.getValuesSourceType();
                     // TODO in the future we can make this more explicit with expectThrows(), when the exceptions are standardized
+                    AssertionError failure = null;
                     try {
                         searchAndReduce(indexSearcher, new MatchAllDocsQuery(), aggregationBuilder, fieldType);
                         if (supportedVSTypes.contains(vst) == false || unsupportedMappedFieldTypes.contains(fieldType.typeName())) {
-                            fail("Aggregator [" + aggregationBuilder.getType() + "] should not support field type ["
+                            failure = new AssertionError("Aggregator [" + aggregationBuilder.getType() + "] should not support field type ["
                                 + fieldType.typeName() + "] but executing against the field did not throw an exception");
                         }
                     } catch (Exception | AssertionError e) {
                         if (supportedVSTypes.contains(vst) && unsupportedMappedFieldTypes.contains(fieldType.typeName()) == false) {
-                            throw new AssertionError("Aggregator [" + aggregationBuilder.getType() + "] supports field type ["
+                            failure = new AssertionError("Aggregator [" + aggregationBuilder.getType() + "] supports field type ["
                                 + fieldType.typeName() + "] but executing against the field threw an exception: [" + e.getMessage() + "]",
                                 e);
                         }
                     }
+                    if (failure != null) {
+                        throw failure;
+                    }
                 }
             }
         }

+ 2 - 0
x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java

@@ -136,6 +136,8 @@ public class HistoBackedAvgAggregatorTests extends AggregatorTestCase {
         // Note: this is the same list as Core, plus Analytics
         return List.of(
             CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE,
             AnalyticsValuesSourceType.HISTOGRAM
         );
     }

+ 2 - 0
x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java

@@ -136,6 +136,8 @@ public class HistoBackedSumAggregatorTests extends AggregatorTestCase {
         // Note: this is the same list as Core, plus Analytics
         return List.of(
             CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE,
             AnalyticsValuesSourceType.HISTOGRAM
         );
     }

+ 3 - 0
x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java

@@ -140,6 +140,9 @@ public class HistoBackedValueCountAggregatorTests extends AggregatorTestCase {
             CoreValuesSourceType.BYTES,
             CoreValuesSourceType.GEOPOINT,
             CoreValuesSourceType.RANGE,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE,
+            CoreValuesSourceType.IP,
             AnalyticsValuesSourceType.HISTOGRAM
         );
     }

+ 30 - 3
x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregatorTests.java

@@ -14,7 +14,10 @@ import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.time.DateUtils;
 import org.elasticsearch.index.fielddata.ScriptDocValues;
+import org.elasticsearch.index.mapper.BooleanFieldMapper;
+import org.elasticsearch.index.mapper.DateFieldMapper;
 import org.elasticsearch.index.mapper.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
@@ -62,16 +65,40 @@ public class TTestAggregatorTests extends AggregatorTestCase {
 
     @Override
     protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldType, String fieldName) {
+        if (fieldType instanceof NumberFieldMapper.NumberFieldType) {
+            return new TTestAggregationBuilder("foo")
+                .a(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
+                    .setFilter(QueryBuilders.rangeQuery(fieldName).lt(10)).build())
+                .b(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
+                    .setFilter(QueryBuilders.rangeQuery(fieldName).gte(10)).build());
+        } else if (fieldType.typeName().equals(DateFieldMapper.CONTENT_TYPE)
+            || fieldType.typeName().equals(DateFieldMapper.DATE_NANOS_CONTENT_TYPE)) {
+
+            return new TTestAggregationBuilder("foo")
+                .a(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
+                    .setFilter(QueryBuilders.rangeQuery(fieldName).lt(DateUtils.toInstant(10))).build())
+                .b(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
+                    .setFilter(QueryBuilders.rangeQuery(fieldName).gte(DateUtils.toInstant(10))).build());
+        } else if (fieldType.typeName().equals(BooleanFieldMapper.CONTENT_TYPE)) {
+            return new TTestAggregationBuilder("foo")
+                .a(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
+                    .setFilter(QueryBuilders.rangeQuery(fieldName).lt("true")).build())
+                .b(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
+                    .setFilter(QueryBuilders.rangeQuery(fieldName).gte("false")).build());
+        }
+        // if it's "unsupported" just use matchall filters to avoid parsing issues
         return new TTestAggregationBuilder("foo")
             .a(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
-                .setFilter(QueryBuilders.rangeQuery(fieldName).lt(10)).build())
+                .setFilter(QueryBuilders.matchAllQuery()).build())
             .b(new MultiValuesSourceFieldConfig.Builder().setFieldName(fieldName)
-                .setFilter(QueryBuilders.rangeQuery(fieldName).gte(10)).build());
+                .setFilter(QueryBuilders.matchAllQuery()).build());
     }
 
     @Override
     protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
-        return List.of(CoreValuesSourceType.NUMERIC);
+        return List.of(CoreValuesSourceType.NUMERIC,
+            CoreValuesSourceType.BOOLEAN,
+            CoreValuesSourceType.DATE);
     }
 
     @Override

+ 2 - 1
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java

@@ -33,6 +33,7 @@ import org.elasticsearch.search.aggregations.bucket.geogrid.GeoGridAggregationBu
 import org.elasticsearch.search.aggregations.bucket.geogrid.InternalGeoGrid;
 import org.elasticsearch.search.aggregations.bucket.geogrid.InternalGeoGridBucket;
 import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper;
+import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
 import org.elasticsearch.search.aggregations.support.ValuesSourceType;
 import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin;
 import org.elasticsearch.xpack.spatial.index.fielddata.CentroidCalculator;
@@ -93,7 +94,7 @@ public abstract class GeoShapeGeoGridTestCase<T extends InternalGeoGridBucket<T>
 
     @Override
     protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
-        return List.of(GeoShapeValuesSourceType.instance());
+        return List.of(GeoShapeValuesSourceType.instance(), CoreValuesSourceType.GEOPOINT);
     }
 
     @Override