-
Notifications
You must be signed in to change notification settings - Fork 741
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
Fixing Flaky Datetime Test with Hypothesis #4212
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it changes the fundamental nature of this test. I'm also not sure what the relevance of the reference to mktime
is; what is using mktime
?
@@ -227,12 +231,11 @@ def test_datetime_typeerror(): | |||
|
|||
|
|||
@given(dt=st.datetimes(MIN_DATETIME, MAX_DATETIME)) | |||
@example(dt=pdt.datetime(1971, 1, 2, 0, 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a random case at first. Can add it back in if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The datetime tested here is equal to the previous value of MIN_DATETIME
value on Windows (see line 59 above). I assume that's why it was special cased.
From here: https://docs.python.org/3/library/datetime.html#datetime.datetime.fromtimestamp the underlying C library will be called, and it seems that If those are wrong assumptions, feel free to provide the correct explanation as to what is under the hood to determine the range. |
The discussion here has been very useful and I think clarified the situation for me - I opened #4212 which is based upon this and hopefully is a solution we all like? |
Attempt to close #3922
According to this: https://stackoverflow.com/a/78163597
mktime
in C library on Windows can produce unexpected results if the local timezone is in DST. If we generate test cases from Hypothesis with a range, we should fix the timezone to UTC. Separate tests can be created with multiple timezones with reduced ranges.