add tests to SumAggregatorTests #53568
Conversation
This adds tests for supported ValuesSourceTypes, unmapped fields, scripting, and the missing param. The tests for unmapped fields and scripting are migrated from the SumIT integration test
andyb-elastic
added
>test
|
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
| assertThat(sum.getValue(), equalTo(0.0)); | ||
| } | ||
|
|
||
| /** This test has been moved to {@link SumAggregatorTests#testUnmapped()} */ |
There was a problem hiding this comment.
These are because this class still extends AbstractNumericTestCase - the remainder of the tests will migrate in another pr
| import static java.util.Collections.singletonList; | ||
| import static java.util.Collections.singletonMap; | ||
| import static java.util.stream.Collectors.toList; | ||
| import static org.elasticsearch.common.lucene.search.Queries.newMatchAllQuery; |
There was a problem hiding this comment.
Personally I don't think this is an improvement.
There was a problem hiding this comment.
Yeah it's probably a net negative on readability
| .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, FIELD_SCRIPT_NAME, singletonMap("field", FIELD_NAME)))); | ||
| } | ||
|
|
||
| private void scriptTestCase(int numValuesPerField, SumAggregationBuilder builder) throws IOException { |
There was a problem hiding this comment.
I don't think this name is right. It looks like it is just a generic "does random stuff reduce right" kind of thing.
There was a problem hiding this comment.
Had the same thought after writing it - the script specific part is the expectedSum = sum + (numDocs * numValuesPerField); because it expects the script to increment all the fields by 1
I'll see if I can make it less confusing
There was a problem hiding this comment.
I made it a little more general and moved some other test cases to it too. Still not thrilled with its readability
nik9000
left a comment
nik9000
left a comment
There was a problem hiding this comment.
LGTM. Incremental improvement!
|
Thanks! |
This adds tests for supported ValuesSourceTypes, unmapped fields, scripting, and the missing param. The tests for unmapped fields and scripting are migrated from the SumIT integration test
This adds tests for supported ValuesSourceTypes, unmapped fields, scripting, and the missing param. The tests for unmapped fields and scripting are migrated from the SumIT integration test
4 participants
This adds tests for supported ValuesSourceTypes, unmapped fields,
scripting, and the missing param. The tests for unmapped fields and
scripting are migrated from the SumIT integration test