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.

Sign up for GitHub

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

Jump to bottom

Add more unit tests for terms aggregation #24949

Merged
merged 1 commit into from
May 31, 2017
Merged

Add more unit tests for terms aggregation #24949

merged 1 commit into from
May 31, 2017

Conversation

martijnvg
Copy link
Member

and removed terms agg java integ tests that were replaced by unit tests.

Relates to #22278

@martijnvg martijnvg added :Analytics/Aggregations Aggregations review >test Issues or PRs that are addressing/adding tests v6.0.0 labels May 30, 2017
@martijnvg martijnvg mentioned this pull request May 30, 2017
Closed
63 tasks
jpountz
jpountz reviewed May 30, 2017
View reviewed changes
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

The new tests look good but I'm not totally comfortable with removing the integ tests yet.

jpountz
jpountz approved these changes May 30, 2017
View reviewed changes
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

Actually I just discussed it with Martijn and I think his approach is right: the actual integ tests for aggs should be rest tests.

@martijnvg
Added more unit test coverage for terms aggregation and
a089dc9 
 removed terms agg integration tests that were replaced by unit tests.
@martijnvg martijnvg merged commit a089dc9 into elastic:master May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpountz jpountz jpountz approved these changes

Assignees
No one assigned
Labels
:Analytics/Aggregations Aggregations >test Issues or PRs that are addressing/adding tests v6.0.0-alpha2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@martijnvg @jpountz @clintongormley