[Tests] Fix rare edge case in SimpleQueryStringBuilderTests #35201
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the random query string is "now" by accident _and_ we are also not setting some field names to use explicitely, then we can hit the "mapped_date" field from default test setup. This correctly leads to the query being was marked as not cacheable, but we assume and check so later. This change fixes this rare edge case by making sure we don't hit the "date" field in this rare cases. Closes elastic#35183
cbuescher
added
>test
|
Pinging @elastic/es-search-aggs |
cbuescher
commented
Nov 2, 2018
| // cacheable and trigger later test failures (see https://github.com/elastic/elasticsearch/issues/35183) | ||
| if (fieldCount == 0 && result.value().equalsIgnoreCase("now")) { | ||
| fields.put(STRING_FIELD_NAME_2, 2.0f / randomIntBetween(1, 20)); | ||
| } |
There was a problem hiding this comment.
I don't like really like adding so many lines for a relatively rare case, but every other way (e.g. disallowing "now" altogether as a query string) would have been worse IMHO.
nik9000
approved these changes
Nov 2, 2018
cbuescher
pushed a commit
that referenced
this pull request
Nov 5, 2018
If the random query string is "now" by accident _and_ we are also not setting some field names to use explicitely, then we can hit the "mapped_date" field from default test setup. This correctly leads to the query being was marked as not cacheable, but we assume and check so later. This change fixes this rare edge case by making sure we don't hit the "date" field in this rare cases. Closes #35183
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Dec 13, 2018
A previous fix of a similar problem in elastic#35201 wasn't general enough, we also need to catch cases where the randomly generated query string starts with some version of "now" and hits a date field. Closes elastic#36595
cbuescher
pushed a commit
that referenced
this pull request
Dec 14, 2018
cbuescher
pushed a commit
that referenced
this pull request
Dec 14, 2018
cbuescher
pushed a commit
that referenced
this pull request
Dec 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the random query string is "now" by accident and we are also not setting
some field names to use explicitely, then we can hit the "mapped_date" field
from default test setup. This correctly leads to the query being was marked as
not cacheable, but we assume and check so later. This change fixes this rare
edge case by making sure we don't hit the "date" field in this rare cases.
Closes #35183