honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

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.
This commit is contained in:
Zachary Tong 2020-06-01 12:10:05 -04:00 committed by GitHub
parent 4dd05ccf9e
commit daaf5a3dcc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 72 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTextAggregatorTests.java
View File

@ -84,7 +84,7 @@ public class SignificantTextAggregatorTests extends AggregatorTestCase {
sigAgg.sourceFieldNames(Arrays.asList(new String [] {"json_only_field"}));
}
SamplerAggregationBuilder aggBuilder = new SamplerAggregationBuilder("sampler")
.subAggregation(sigAgg);
.subAggregation(sigAgg);
try (IndexReader reader = DirectoryReader.open(w)) {
assertEquals("test expects a single segment", 1, reader.leaves().size());

4
server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java
View File

@ -704,8 +704,8 @@ public class AvgAggregatorTests extends AggregatorTestCase {
protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
return Arrays.asList(
CoreValuesSourceType.NUMERIC,
CoreValuesSourceType.DATE,
CoreValuesSourceType.BOOLEAN);
CoreValuesSourceType.BOOLEAN,
CoreValuesSourceType.DATE);
}
@Override

4
server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java
View File

@ -164,8 +164,8 @@ public class MaxAggregatorTests extends AggregatorTestCase {
protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
return Arrays.asList(
CoreValuesSourceType.NUMERIC,
CoreValuesSourceType.DATE,
CoreValuesSourceType.BOOLEAN
CoreValuesSourceType.BOOLEAN,
CoreValuesSourceType.DATE
);
}

7
server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java
View File

@ -475,10 +475,9 @@ public class StatsAggregatorTests extends AggregatorTestCase {
@Override
protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
return Arrays.asList(
CoreValuesSourceType.NUMERIC,
CoreValuesSourceType.DATE,
CoreValuesSourceType.BOOLEAN);
return Arrays.asList(CoreValuesSourceType.NUMERIC,
CoreValuesSourceType.BOOLEAN,
CoreValuesSourceType.DATE);
}
@Override

4
server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java
View File

@ -403,8 +403,8 @@ public class SumAggregatorTests extends AggregatorTestCase {
protected List<ValuesSourceType> getSupportedValuesSourceTypes() {
return Arrays.asList(
CoreValuesSourceType.NUMERIC,
CoreValuesSourceType.DATE,
CoreValuesSourceType.BOOLEAN);
CoreValuesSourceType.BOOLEAN,
CoreValuesSourceType.DATE);
}
@Override

5
server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountAggregatorTests.java
View File

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

25
test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java
View File

@ -188,7 +188,7 @@ public abstract class AggregatorTestCase extends ESTestCase {
// Make this @Before instead of @BeforeClass so it can call the non-static getSearchPlugins method
@Before
public void initValuesSourceRegistry() {
SearchModule searchModule = new SearchModule(Settings.EMPTY, false, getSearchPlugins());
SearchModule searchModule = new SearchModule(Settings.EMPTY, false, this.getSearchPlugins());
valuesSourceRegistry = searchModule.getValuesSourceRegistry();
}
@ -327,7 +327,8 @@ public abstract class AggregatorTestCase extends ESTestCase {
.collect(Collectors.toMap(MappedFieldType::name, Function.identity())));
fieldNameToType.putAll(getFieldAliases(fieldTypes));
registerFieldTypes(searchContext, mapperService, fieldNameToType);
registerFieldTypes(searchContext, mapperService,
fieldNameToType);
doAnswer(invocation -> {
/* Store the release-ables so we can release them at the end of the test case. This is important because aggregations don't
* close their sub-aggregations. This is fairly similar to what the production code does. */
@ -721,18 +722,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 e) {
} catch (Exception | AssertionError e) {
if (supportedVSTypes.contains(vst) && unsupportedMappedFieldTypes.contains(fieldType.typeName()) == false) {
fail("Aggregator [" + aggregationBuilder.getType() + "] supports field type ["
+ fieldType.typeName() + "] but executing against the field threw an exception: [" + e.getMessage() + "]");
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;
}
}
}
}
@ -768,8 +774,8 @@ public abstract class AggregatorTestCase extends ESTestCase {
json = "{ \"" + fieldName + "\" : \"" + f + "\" }";
} else {
// smallest numeric is a byte so we select the smallest
v = Math.abs(randomByte());
json = "{ \"" + fieldName + "\" : \"" + v + "\" }";
v = Math.abs(randomByte());
json = "{ \"" + fieldName + "\" : \"" + v + "\" }";
}
doc.add(new SortedNumericDocValuesField(fieldName, v));
@ -836,8 +842,7 @@ public abstract class AggregatorTestCase extends ESTestCase {
" \"gte\" : \"" + start + "\",\n" +
" \"lte\" : \"" + end + "\"\n" +
" }}";
} else if (fieldType.getValuesSourceType().equals(CoreValuesSourceType.GEOPOINT)) {
} else if (vst.equals(CoreValuesSourceType.GEOPOINT)) {
double lat = randomDouble();
double lon = randomDouble();
doc.add(new LatLonDocValuesField(fieldName, lat, lon));

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

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

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

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

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

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

40
x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregatorTests.java
View File

@ -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;
@ -34,11 +37,14 @@ import org.elasticsearch.search.aggregations.bucket.global.InternalGlobal;
import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.histogram.InternalHistogram;
import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.MultiValuesSourceFieldConfig;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
import org.elasticsearch.search.lookup.LeafDocLookup;
import org.elasticsearch.xpack.analytics.AnalyticsPlugin;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@ -60,13 +66,43 @@ 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 Arrays.asList(CoreValuesSourceType.NUMERIC,
CoreValuesSourceType.BOOLEAN,
CoreValuesSourceType.DATE);
}
@Override
protected ScriptService getMockScriptService() {
Map<String, Function<Map<String, Object>, Object>> scripts = new HashMap<>();