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

Fixing Flaky Datetime Test with Hypothesis #4212

Closed
wants to merge 3 commits into from

Conversation

Cheukting
Copy link
Contributor

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.

Copy link
Member

@pganssle pganssle left a 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))
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this example?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@Cheukting
Copy link
Contributor Author

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?

From here: https://docs.python.org/3/library/datetime.html#datetime.datetime.fromtimestamp the underlying C library will be called, and it seems that mktime is being used.

If those are wrong assumptions, feel free to provide the correct explanation as to what is under the hood to determine the range.

@davidhewitt
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-skip-changelog Skip checking changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

datetime tests out of range macOS & python 3.12
4 participants