Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more tests for rate aggregation #95576

Merged
merged 2 commits into from
Apr 26, 2023

Conversation

kkrik-es
Copy link
Contributor

The missing tests were derived from rate aggregation documentation:

  • Composite aggregation
  • Rate with mode set to value_count
  • Aggregation on a runtime field using script

Related to #26220

The missing tests were derived from [rate aggregation documentation](
https://www.elastic.co/guide/en/elasticsearch/reference/current/search
-aggregations-metrics-rate-aggregation.html).

Related to elastic#26220.
@kkrik-es kkrik-es added >test Issues or PRs that are addressing/adding tests :Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Apr 26, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@elasticsearchmachine elasticsearchmachine added v8.9.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Apr 26, 2023
@kkrik-es kkrik-es removed the external-contributor Pull request authored by a developer outside the Elasticsearch team label Apr 26, 2023
@elasticsearchmachine elasticsearchmachine merged commit 5df03a9 into elastic:main Apr 26, 2023
@kkrik-es kkrik-es deleted the fix/26220 branch April 27, 2023 05:48
@salvatore-campagna
Copy link
Contributor

@kkrik-es LGTM...I would only consider using close_to when comparing floating point values.

@kkrik-es
Copy link
Contributor Author

Good point, thanks Salvatore. Since the previous version didn't use it, I'm inclined it leave it as is (but apply it in future changes), but I'm happy to create a PR if you feel strongly about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants