Address ready_to_verify_presenter_spec flaky tests.#9652
Merged
JackRyan1989 merged 3 commits intomainfrom Dec 1, 2023
Merged
Conversation
added 3 commits
November 22, 2023 14:54
…y from DST in south america
…o-verify-presenter-flaky-test
n1zyy
reviewed
Nov 22, 2023
| let(:created_at) { described_class::USPS_SERVER_TIMEZONE.parse('2023-06-14T00:00:00Z') } | ||
| let(:enrollment_established_at) do | ||
| described_class::USPS_SERVER_TIMEZONE.parse('2022-08-14T00:00:00Z') | ||
| described_class::USPS_SERVER_TIMEZONE.parse('2023-07-14T00:00:00Z') |
Contributor
There was a problem hiding this comment.
The purpose of these changes is not obvious to me. Was this ultimately just to get us away from an edge-of-DST-change date?
Contributor
Author
There was a problem hiding this comment.
That's exactly the case. As far as I can tell, September 11- September 12th is DST change for UTC -300. So changing by a year and a month.
soniaconnolly
approved these changes
Nov 24, 2023
Contributor
soniaconnolly
left a comment
There was a problem hiding this comment.
LGTM. Thanks for tracking this down! As you noted, the code should probably also be changed to be less sensitive to DST.
Merged
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
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.
What does this pull request fix?
The time zone for the due date calculation for
in_person_enrollmentshifts ahead one hour very rarely. Seemingly always on 9/12/22 and always between -400 and -300.This causes test flakiness in
ready_to_verify_presenter_spec.rb.This corresponds pretty well to when Chile decided to delay the start of DST:
https://www.timeanddate.com/news/time/chile-dst-delay-2022.html
🛠 Summary of changes
created_atandenrollment_established_atdates to avoid issues with DST across all timezones..utcto keep timezones consistent.Testing:
Note:
I think this code is still buggy. These changes should remove the flakiness with this particular test, but we need to handle date calculation for when DST strikes and the distance between days increases or decreases by an hour.