Adds unit test for sampler aggregation #23243
Conversation
nik9000
added
review
>test
| /** | ||
| * Unwrap an aggregator. Used for testing. | ||
| */ | ||
| public static Aggregator unwrap(Aggregator in) { |
There was a problem hiding this comment.
I'm not super comfortable about exposing this but I can't think of a much better way to actually close the aggregations during testing without reworking aggregations to make parent aggs close their subaggregations.
There was a problem hiding this comment.
Maybe we could do searchContext.clearReleasables(Lifetime.PHASE) like the main code does since all aggregators are supposed to register themselves on to the search context.
| when(queryShardContext.fieldMapper(fieldType.name())).thenReturn(fieldType); | ||
| when(queryShardContext.getForField(fieldType)).thenReturn(fieldData); | ||
| when(queryShardContext.getForField(fieldType)).then(invocation -> fieldType.fielddataBuilder().build( |
There was a problem hiding this comment.
This blew up without fielddata enabled so I made it return when used instead of all the time.
| /** | ||
| * Unwrap an aggregator. Used for testing. | ||
| */ | ||
| public static Aggregator unwrap(Aggregator in) { |
There was a problem hiding this comment.
Maybe we could do searchContext.clearReleasables(Lifetime.PHASE) like the main code does since all aggregators are supposed to register themselves on to the search context.
| when(queryShardContext.fieldMapper(fieldType.name())).thenReturn(fieldType); | ||
| when(queryShardContext.getForField(fieldType)).thenReturn(fieldData); | ||
| when(queryShardContext.getForField(fieldType)).then(invocation -> fieldType.fielddataBuilder().build( |
| Sampler sampler = searchAndReduce(searcher, new TermQuery(new Term("text", "good")), aggBuilder, textFieldType, | ||
| numericFieldType); | ||
| Min min = sampler.getAggregations().get("min"); | ||
| assertEquals(5.0, min.getValue(), Double.MIN_NORMAL); |
There was a problem hiding this comment.
s/Double.MIN_NORMAL/0/ ? the computation of the min value should not be subject to accuracy issues.
|
@jpountz, I pushed the changes you asked for. Kind of. Have a look? |
Thanks! |
|
Grumble grumble, it looks like this merged instead of squashed, rebased, and merged. I must have lost my cookie.... Sorry! |
|
Backported to 5.x with d73d89c |
Relates to #22278