Refactor MatchAllQueryBuilder, TermQueryBuilder, IdsQueryBuilder #10454
Conversation
| context = new QueryParseContext(index, queryParserService); | ||
| testQuery = createTestQuery(); | ||
| String contentString = createXContent(testQuery).string(); | ||
| parser = XContentFactory.xContent(contentString).createParser(contentString); |
There was a problem hiding this comment.
Maybe it would be easier to extend ElasticsearchSingleNodeTest so that all these bindings are already done for you?
There was a problem hiding this comment.
Thanks, I thought it nicer to have unit tests here than having to do the whole node setup. I'll try this, but how expensive is the setup of the ElasticsearchSingleNodeTest?
If we write more query tests like this I, an alternative would be to have an own BaseTest class for this. Already suggested this to Lee.
|
I left a bunch of comments. I am pretty positive that we can trim down the code needed for testing, by just having a base test class with a few abstract methods that provide the query to test etc. we'll see how far we can take this. I am happy that we are going with unit testing rather than requiring to fire up a node. My personal preference, I think a single PR per query would be easier to review, also then you don't have to adapt all of the other queries to the same comments :) but I see how this time it was better to touch a few of them rather than just one. |
| @@ -88,6 +94,10 @@ public IdsQueryBuilder boost(float boost) { | |||
| return this; | |||
| } | |||
|
|
|||
| public float getBoost() { | |||
There was a problem hiding this comment.
one more thing, these are going to be user facing public methods, we should add javadocs for each of them
|
I worked through your comments. When we got through the other PR with Lees query we can start identifying common issues and test setup that can be generalised. I'll wait with further improvements here till we decided what kind of Parser/Builder merge we want on the feature branch and if we have to rebase there. |
clintongormley
added
:Search/Search
MatchAllQueryBuilder, TermQueryBuilder, IdsQueryBuilder now include the fromXContent, doXContent and toQuery methods suggested in #9901. Also implementing the Streamable interface and adding unit tests for the serialization and parsing.
NOTE: this PR is against the feature/query-parse-refactoring branch.