SQL: improve timezone sensitive JDBC tests to account for offset differences #51592
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
* Changed several tests to compare only the relevant parts of the conversion failure error message and ignore the actual datetime String from the error message itself
astefan
added
>bug
>test
|
Pinging @elastic/es-search (:Search/SQL) |
matriv
approved these changes
Jan 29, 2020
| doWithQuery(SELECT_ALL_FIELDS, (results) -> { | ||
| // Create simple timezones (in the form of Z/GMT+/-x so that timezones offsets differences between | ||
| // Java versions (the one server runs on and the one the test runs on) will not skew the tests' results | ||
| int hoursOffset = randomIntBetween(0, 18); |
There was a problem hiding this comment.
Could you please add a comment here, for the 18 (maximum offset)?
bpintea
reviewed
Jan 29, 2020
| int totalSeconds = (hoursOffset * 3600 + subHours * 60) * plusMinus; | ||
|
|
||
| ZoneOffset randomFixedZone = ZoneOffset.ofTotalSeconds(totalSeconds); | ||
| String zoneString = (hoursOffset == 0 && subHours == 0) ? "Z" : "GMT" + randomFixedZone.getId(); |
There was a problem hiding this comment.
Just a note: while GMT and UTC are identical in this timezones context, my impression is that there is a shift towards using the UTC for references.
This was referenced Jan 29, 2020
matriv
added a commit
that referenced
this pull request
May 12, 2020
What basically happens is that the server runs on a java version where a certain timezone has a set of offset rules, while the test itself runs on a java version where the same timezone has changed offset rules (think about a country deciding to use utc+x instead of utc+y). With this change the timezone used in testGettingTimeWithoutCalendar is randomly generated by creating a fixed offset timezone (ie GMT+X or Z instead of Australia/West timezone) so that the offset is always fixed, thus avoiding the described issue. Similar fix to #51592 Fixes: #52650
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.
Original PR: #51290
Based on the discussion here decision has been made to merge this PR in 6.8 only.
This PR addresses CI failures reported in #45035 and #48234, but also other even more rare CI failures looking like
What basically happens is that the server runs on a java version where a certain timezone has a set of offset rules, while the test itself runs on a java version where the same timezone has changed offset rules (think about a country deciding to use utc+x instead of utc+y).
This PR changed several tests to compare only the relevant parts of the conversion failure error message and ignore the actual datetime String from the error message itself (thus, ignoring the value of the datetime itself that's part of the error message).
Also, there is another change fixing the second issue mentioned above where a timezone in
testGettingDateWithoutCalendaris randomly generated by creating a fixed offset timezone (ieGMT+XorZinstead ofAustralia/Westtimezone) so that the offset is always fixed.