Skip to content

Conversation

@andyb-elastic
Copy link
Contributor

Moves the remaining tests for the missing aggregation into its AggregatorTestCase out of its integration test and remove the IT. Figured this was worth doing while working on coverage in this area

For #42893

Move the remaining tests for the missing aggregation into its
AggregatorTestCase out of its integration test and remove the IT

For elastic#42893
@andyb-elastic andyb-elastic added >test Issues or PRs that are addressing/adding tests :Analytics/Aggregations Aggregations v8.0.0 labels Mar 12, 2020
@elasticmachine
Copy link
Collaborator

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

}
},
internalMissing -> {
(Consumer<InternalMissing>) internalMissing -> {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this, but could you write (InternalMissing internalMissing) -> { instead of the cast?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is much better, thanks

final IndexSearcher indexSearcher = newSearcher(indexReader, true, true);
final MappedFieldType[] fieldTypesArray = fieldTypes.toArray(new MappedFieldType[0]);
final InternalMissing missing;
final I missing;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call it result instead of missing because it isn't always InternalMissing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

👍

@andyb-elastic andyb-elastic merged commit f6d3784 into elastic:master Mar 13, 2020
andyb-elastic added a commit to andyb-elastic/elasticsearch that referenced this pull request Apr 1, 2020
Move the remaining tests for the missing aggregation into its
AggregatorTestCase out of its integration test and remove the IT
andyb-elastic added a commit that referenced this pull request Apr 2, 2020
Move the remaining tests for the missing aggregation into its
AggregatorTestCase out of its integration test and remove the IT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations >test Issues or PRs that are addressing/adding tests v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants