Skip to content

Conversation

@not-napoleon
Copy link
Member

This PR provides tests for the behavior of the range fields on aggregations that use ValuesSourceType.ANY. This PR doesn't add support for ranges, merely checks that aggregations which we expect to "just work" with ranges do, and that the behavior of other "Any" type aggregations is consistent.

  • Missing Aggregation - Works with range fields - test included
  • Cardinality Aggregation - Works with range fields - test included
  • Value Count Aggregation - Works with range fields - test included
  • Terms Aggregation - Throws AggregationExecutionException on range fields - test included
  • Significant Terms Aggregation - Throws AEE - test included
  • Rare Terms Aggregation - Throws AEE - test included
  • Diversified Aggregation - Throws AEE - test not included

I didn't add a test for Diversified because (a) it's very clear what the behavior is from the code and (b) it's under-tested as is, so it'd be a lot of code to add in tests for this. I still might, but wanted to get the easy tests up for review first.

@not-napoleon not-napoleon added >non-issue >test Issues or PRs that are addressing/adding tests :Analytics/Aggregations Aggregations labels Jul 17, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

Copy link
Contributor

@polyfractal polyfractal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@not-napoleon not-napoleon merged commit 403ee95 into elastic:feature-range-aggregations Jul 26, 2019
@not-napoleon not-napoleon deleted the feature/range-any-tests branch July 26, 2019 14:30
@polyfractal polyfractal mentioned this pull request Jul 26, 2019
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations >non-issue >test Issues or PRs that are addressing/adding tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants