-
Notifications
You must be signed in to change notification settings - Fork 30
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
Tests: Use UTC-based utcnow().date()
instead of localtime-based date.today()
#441
Conversation
ee86c14
to
07b8d79
Compare
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.
Other than the test addition this looks good to me.
The commit message could be clarified a bit. Using UTC is not automatically better - it depends on the use case.
What is a problem is that we were comparing utc variants with local variants in
crate-python/src/crate/client/sqlalchemy/doctests/itests.txt
Lines 70 to 84 in a849cea
>>> from datetime import datetime | |
>>> now = datetime.utcnow() | |
>>> dt = location.date | |
>>> dt.year == now.year | |
True | |
>>> dt.month == now.month | |
True | |
>>> dt.day == now.day | |
True | |
>>> (now - location.datetime_tz).seconds < 4 | |
True |
That is the problem and caused test flakyness.
src/crate/testing/test_datetime.py
Outdated
import time_machine | ||
|
||
|
||
class DateTodayTest(TestCase): |
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.
Please remove, there's no need to create tests that verify documented behavior of the standard library.
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.
Thanks. I've removed the test cases and amended the commit message.
`datetime.date.today()` returns the current local date. In order to reduce test flakyness when comparing UTC-based datetimes with localtime- based variants, `datetime.utcnow().date()` is a better choice.
07b8d79
to
4cf9d00
Compare
Motivation
utcnow()
andutcfromtimestamp()
for Python 3 #440, which went too far.About
datetime.date.today()
returns the current local date. So, its outcome depends on the system time zone. Because we want to use UTC-based date and time objects instead,datetime.utcnow().date()
is a better choice.