Fix local timezone and timezone related conversions #526
Conversation
now(tz) should return the same time utcnow returns adjusted by whatever offset is contained by tz. Currently, the offset to freeze_time is also added, which is removed by this change The current unit test is wrong because the UTC time is 2:00:00, so GMT5 should be 7:00:00, not 3:00:00 Closes spulec#405
Fix now(tz)
There was a problem hiding this comment.
Hi @priezz, thaank you for the PR! At a first glance, this seems mostly sensible - but I would have to do a second (/third) deep-dive, as there seem to be quite a few functional changes.
Can you extend the tests to account for all changes? That makes it easier to verify that everything works as expected.
Sure, if after the deep dive you will confirm that the changes make sense, I will add missing/update existing tests. |
| @@ -39,6 +40,8 @@ | |||
| real_date = datetime.date | |||
| real_datetime = datetime.datetime | |||
| real_date_objects = [real_time, real_localtime, real_gmtime, real_monotonic, real_perf_counter, real_strftime, real_date, real_datetime] | |||
| real_tzlocal = dateutil.tz.tzlocal | |||
| real_tz_env = os.environ["TZ"] if "TZ" in os.environ else '' | |||
There was a problem hiding this comment.
| real_tz_env = os.environ["TZ"] if "TZ" in os.environ else '' | |
| real_tz_env = os.environ.get("TZ", "") |
| current_time = get_current_time() | ||
| return calendar.timegm(current_time.timetuple()) + current_time.microsecond / 1000000.0 | ||
|
|
||
| current_time_utc = get_current_time() - datetime.timedelta(seconds=tz_offsets[-1].total_seconds()) |
There was a problem hiding this comment.
| current_time_utc = get_current_time() - datetime.timedelta(seconds=tz_offsets[-1].total_seconds()) | |
| current_time_utc = get_current_time() - tz_offsets[-1] |
tz_offsets contains a list of datetime.timedelta-objects, so this is functionally equivalent as far as I can see
|
|
||
|
|
||
| def fake_gmtime(t=None): | ||
| if t is not None: | ||
| return real_gmtime(t) | ||
| if _should_use_real_time(): | ||
| return real_gmtime() | ||
| return get_current_time().timetuple() | ||
|
|
||
| current_time_utc = get_current_time() - datetime.timedelta(seconds=tz_offsets[-1].total_seconds()) |
There was a problem hiding this comment.
Since this would always have to be in UTC, wouldn't this be get_current_time() - UTC_offset (whatever the value for UTC_offset is)?
| @@ -323,9 +342,12 @@ def __sub__(self, other): | |||
|
|
|||
| @classmethod | |||
| def today(cls): | |||
| result = cls._date_to_freeze() + cls._tz_offset() | |||
| result = cls._date_to_freeze() | |||
There was a problem hiding this comment.
I think we do need some sort of timezone offset here, as the day can be different in different timezones.
Was there a specific bug with this operation?
This PR addresses issues with
Fixes #204, #291, #299, #348, #377, #405, #472, #513.
tz_offsetparameter is now considered as a local timezone offsettz_offsetdefaults to the local environment timezonetime_to_freeze, it defaults totz_offsettime.strftimeworks as expected