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

[Test] Extending parsing checks for SearchResponse #25148

Merged

Conversation

cbuescher
Copy link
Member

This change extends the tests and parsing of SearchResponse to make sure we can skip additional fields the parser doesn't know for forward compatibility reasons.

/**
* This SearchResponse doesn't include SearchHits, Aggregations, Suggestions, ShardSearchFailures, SearchProfileShardResults
* to make it possible to only test properties of the SearchResponse itself
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

@javanna @tlrx I'd be in interested in your opinion on this. I think it makes sense to take out some complexity of the test here to defer the more detailed tests for those tests to the respective other unit test.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense too

return new SearchResponse(internalSearchResponse, null, totalShards, successfulShards, tookInMillis, shardSearchFailures);
}

/**
* the "_shard/total/failures" section makes if impossible to directly
Copy link
Member

Choose a reason for hiding this comment

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

makes it

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM

@cbuescher cbuescher force-pushed the extendParsingTests-searchResponse branch from 593644c to e7b5204 Compare June 9, 2017 13:16
@cbuescher cbuescher merged commit 823cbb4 into elastic:master Jun 9, 2017
cbuescher added a commit that referenced this pull request Jun 9, 2017
This change extends the tests and parsing of SearchResponse to make sure we can
skip additional fields the parser doesn't know for forward compatibility
reasons.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 10, 2017
* master: (53 commits)
  Log checkout so SHA is known
  Add link to community Rust Client (elastic#22897)
  "shard started" should show index and shard ID (elastic#25157)
  await fix testWithRandomException
  Change BWC versions on create index response
  Return the index name on a create index response
  Remove incorrect bwc branch logic from master
  Correctly format arrays in output
  [Test] Extending parsing checks for SearchResponse (elastic#25148)
  Scripting: Change keys for inline/stored scripts to source/id (elastic#25127)
  [Test] Add test for custom requests in High Level Rest Client (elastic#25106)
  nested: In case of a single type the _id field should be added to the nested document instead of _uid field.
  `type` and `id` are lost upon serialization of `Translog.Delete`. (elastic#24586)
  fix highlighting docs
  Fix NPE in token_count datatype with null value (elastic#25046)
  Remove the postings highlighter and make unified the default highlighter choice (elastic#25028)
  [Test] Adding test for parsing SearchShardFailure leniently (elastic#25144)
  Fix typo in shards.asciidoc (elastic#25143)
  List Hibernate Search (elastic#25145)
  [DOCS] update maxRetryTimeout in java REST client usage page
  ...
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 10, 2017
* master: (80 commits)
  Test: remove faling test that relies on merge order
  Log checkout so SHA is known
  Add link to community Rust Client (elastic#22897)
  "shard started" should show index and shard ID (elastic#25157)
  await fix testWithRandomException
  Change BWC versions on create index response
  Return the index name on a create index response
  Remove incorrect bwc branch logic from master
  Correctly format arrays in output
  [Test] Extending parsing checks for SearchResponse (elastic#25148)
  Scripting: Change keys for inline/stored scripts to source/id (elastic#25127)
  [Test] Add test for custom requests in High Level Rest Client (elastic#25106)
  nested: In case of a single type the _id field should be added to the nested document instead of _uid field.
  `type` and `id` are lost upon serialization of `Translog.Delete`. (elastic#24586)
  fix highlighting docs
  Fix NPE in token_count datatype with null value (elastic#25046)
  Remove the postings highlighter and make unified the default highlighter choice (elastic#25028)
  [Test] Adding test for parsing SearchShardFailure leniently (elastic#25144)
  Fix typo in shards.asciidoc (elastic#25143)
  List Hibernate Search (elastic#25145)
  ...
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 10, 2017
* master: (1889 commits)
  Test: remove faling test that relies on merge order
  Log checkout so SHA is known
  Add link to community Rust Client (elastic#22897)
  "shard started" should show index and shard ID (elastic#25157)
  await fix testWithRandomException
  Change BWC versions on create index response
  Return the index name on a create index response
  Remove incorrect bwc branch logic from master
  Correctly format arrays in output
  [Test] Extending parsing checks for SearchResponse (elastic#25148)
  Scripting: Change keys for inline/stored scripts to source/id (elastic#25127)
  [Test] Add test for custom requests in High Level Rest Client (elastic#25106)
  nested: In case of a single type the _id field should be added to the nested document instead of _uid field.
  `type` and `id` are lost upon serialization of `Translog.Delete`. (elastic#24586)
  fix highlighting docs
  Fix NPE in token_count datatype with null value (elastic#25046)
  Remove the postings highlighter and make unified the default highlighter choice (elastic#25028)
  [Test] Adding test for parsing SearchShardFailure leniently (elastic#25144)
  Fix typo in shards.asciidoc (elastic#25143)
  List Hibernate Search (elastic#25145)
  ...
@clintongormley clintongormley added v6.0.0-beta1 >test Issues or PRs that are addressing/adding tests and removed v6.0.0 labels Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>test Issues or PRs that are addressing/adding tests v5.6.0 v6.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants