Faked time shouldn't always be the same.
Excerpt from the docs:
"[…] all calls to datetime.datetime.now(), datetime.datetime.utcnow(), […] will return the time that has been frozen."
But this is not the behaviour I expect.
Example code:
from datetime import datetime
def test_something():
print('now(): ', datetime.now())
print('utcnow():', datetime.utcnow())
Output without Freezegun (system timezone is "Europe/Berlin"):
now(): 2015-03-10 00:46:40.465879
utcnow(): 2015-03-09 23:46:40.466031
Output with Freezegun (method decorated with @freeze_time('2012-03-11 19:21:23')):
now(): 2012-03-11 19:21:23
utcnow(): 2012-03-11 19:21:23
However, I expected the output with Freezegun to reflect the offset (one hour, in this case) and, thus, to return two different date/time instances.
Oh, and Freezegun's fake date/time lacks milliseconds compared to the default update. I've tested on Python 3.4.2.
Thank you for opening this. Your point is very valid. I would like to fix this somehow, but have a few thoughts/questions I'm trying to think through. Any feedback appreciated.
- I think we would need to have some sort of backwards-compatibility. No idea how this would work.
- Should the time passed into freezegun be used as the local time or utc time?
- What if the time passed in has a timezone as well or a
tz_offsetis passed?
I'm also quite worried about how the documentation will read.
I think we would need to have some sort of backwards-compatibility. No idea how this would work.
Staying completely backwards-compatible probably won't work.
Should the time passed into freezegun be used as the local time or utc time?
I'd say this should be the local time. If it should be UTC, either a separate function with some _utc suffix should be provided (just like datetime.now() and datetime.utcnow() in the stdlib), or a keyword argument like utc=True or similar could be used.
What if the time passed in has a timezone as well or a tz_offset is passed?
Passing an optional timezone via a keywords argument should be possible. If that is the case and the time value already specifies timezone, a ValueError could be raised.
I agree it should be the local time. Caller can specify UTC as the timezone if they want that.
Actually, it should probably default to UTC, so that developers in different timezones working on the same project won't see different results in their tests.
We ran into this in the wild and fought it for a good while - I have some thoughts on how we might fix and document it. @spulec I'd be happy to fix this in a PR if you could confirm there is a good chance it'll merge.
By "past behavior" I mean now() ~= utcnow() which is the bug. By "future behavior" I mean now() and utcnow() behave correctly when frozen.
- For backwards compatibility - an opt-in switch (both global & kwarg) that enables future behavior. We log a single warning if
utcnow()is used in under freezegun. The warning can have a link to a full doc page. In a pre-decided future version (say when we go 1.0.0) we drop the warning and drop the bug. - Presumption of local vs. utc and naivety - I think the time passed in should be presumed "local" if naive. We have to decide one way or another and disappoint someone. If utc default is undesirable (because you use
now()in code) - the workaround depends on your (and team members') timezone. If local default is undesirable (because you useutcnow()in code) it seems simpler to document a workaround that says "add-00:00to your freeze argument". - If TZ is supplied (which is the presumption of point 2) - you'd have to hold onto that timezone and convert to system local and UTC for both
now()andutcnow().
Whatcha think @spulec?
@bryanhelmig happy to accept a fix.
Agree with thoughts on 1 and 3.
2 makes me a bit nervous. I fear that this would force the majority of users to change their code (that is just a hunch). If we wanted to keep utc default, we could add a local argument to override it. As you noted, we will make people upset either way.
Yeah - people will be upset either way. I think it boils down to which side of the divide is larger? Folks who use now() exclusively? Or folks who use utcnow() exclusively?
Either way - we can respect the system timezone fairly simply with:
import time
time.tzname
If you provide freeze_time('2015-01-16T20:00:00+00:00') or any other time with a timezone - code should work either way.
The tricky part is naive freeze_time('2015-01-16T20:00:00'). People 100% have to workaround this bug now if you mix now() and utcnow() as it diverges - so fixing it means we 100% will break stuff. So, just gotta break stuff in the right direction. :-)
One thing I didn't realize until now is that our existing support for tz offsets already makes the assumption that we are passing the time for utc use. From the README:
@freeze_time("2012-01-14 03:21:34", tz_offset=-4)
def test():
assert datetime.datetime.utcnow() == datetime.datetime(2012, 01, 14, 03, 21, 34)
assert datetime.datetime.now() == datetime.datetime(2012, 01, 13, 23, 21, 34)
If we wanted to keep this all working, would the fix be a matter of setting tz_offset to default to time.timezone / 3600?
Ah - that may be - thanks for the heads up!
I wasn't even aware of tz_offset - so if the datetime string has a TZ in it too, feels like we could set that to tz_offset as well which is more natural and smooths some edges.
I'm traveling this week but I'll slap together a PR when I'm back.
@freeze_time("2012-01-14 03:21:34", tz_offset=-4) def test(): assert datetime.datetime.utcnow() == datetime.datetime(2012, 01, 14, 03, 21, 34) assert datetime.datetime.now() == datetime.datetime(2012, 01, 13, 23, 21, 34)
BTW, did you notice that the above code uses octal values? This works for integers up to 7, but will fail for 8 and 9 (see https://github.com/github/linguist/issues/1939 on why the syntax highlighting of the console output currently doesn't work):
>>> 07
7
>>> 08
File "<input>", line 1
08
^
SyntaxError: invalid token
See here:
- https://www.python.org/dev/peps/pep-3127/
- https://docs.python.org/3/faq/programming.html#how-do-i-specify-hexadecimal-and-octal-integers
Woah @homeworkprod - the more you know!
@bryanhelmig, you're welcome. :)
Is this project still maintained? It would be good to get something merged for this.
Yes, open to a PR with a clean merge and tests passing.
Our solution to this is to use Arrow.utcnow(), which correctly does timezone conversion using the local system time. I'd be willing to put a PR for this, but not sure you guys are comfortable with adding another dependency in requirement.txt.
I disagree with the premise at the start ofthis ticket. It's MUCH nicer if freezegun locks timezone to UTC. The entire reason we use freezegun is to make time predictable in tests, so having it freezing the time but not the timezone would be a big mistake. This is a documentation issue.
@boxed I disagree with this.
If the TZ is locked to UTC, it suddently becomes impossible for me to run tests under different TZ settings. For calendaring apps and alike (which is where I use freezetime), these tests are critical to find issues that only ocurr under certain scenarios.
If you need to lock the TZ to a specific one, having a second argument to @freeze_time would probably be the best compromise.
The tz_offset argument isn't enough for you?
If you want to run with the local timezone the right thing to do would be to freeze_time with the existing timezone imo. Just like if you want to lock the current time you lock to datetime.now().
This ticket is also in direct conflict with https://github.com/spulec/freezegun/issues/204 as far as I can tell. Users seem to expect the time zone to be locked to UTC by default. I would and so does @azmeuk .
I've been trying to make a PR to fix some issues from @azmeuk . It seems all of https://github.com/spulec/freezegun/issues/205 https://github.com/spulec/freezegun/issues/204 https://github.com/spulec/freezegun/issues/209 https://github.com/spulec/freezegun/issues/208 https://github.com/spulec/freezegun/issues/240
and this ticket are basically the same underlying issue. The tickets should be probably be merged.
Users seem to expect the time zone to be locked to UTC by default.
@boxed I personally believe that the system timezone should be respected even when time is frozen. If that's not deemed appropriate I guess freeze_time could be adapted to accept a tzinfo object that would be used to localize dates and times. If users really expect the simulated system timezone to change to UTC on time freezing this tzinfo kwarg could default to UTC.
By the way, the tzoffset argument should really be deprecated as it makes little sense in the marvellous world of DST and timezone rules changing every now and then . A good deprecation path would be to error out if both tzoffset and tzinfo are provided and convert tzoffset to a fixed offset tzinfo while raising a warning during the deprecation period.
@charettes For me the point of freeze_time is that you create a world where time related things are predictable and portable across machines. Leaving the time zone as is will not satisfy this demand.
I'm all for having a nicer way to freeze but still respect the time zone if that's what you want though. I'd also like a better way to "freeze" and still respect the local time! This is super useful to initialize the monkey patching just once for your entire test suite for example. We do this at work (and we use my fork that is also an active PR) and it has some pretty dramatic performance advantages if you freeze time a lot.
I'd suggest this:
freeze_time('local', tzinfo='local', tick=True)for respecting time zone, local time and keeping the clock movingfreeze_time('2001-01-02', tzinfo='local')for respecting local time zone but locking the restfreeze_time('2001-01-02')for locking everything
...as for tzinfo vs tzoffset I'm all for it. The only problem I see with it is that if you specify a time zone by name that can mean something different at some later point. tzoffset doesn't have this problem, but it also doesn't model DST properly, so I think maybe a third option is needed here? In addition to your suggestion that is.
Hey @boxed.
We do this at work (and we use my fork that is also an active PR) and it has some pretty dramatic performance advantages if you freeze time a lot.
I'd be interested in more details about that. We use freeze_time quite extensively and have noticed significant speedups since we moved to 0.3.10 which includes https://github.com/spulec/freezegun/pull/175.
Any thoughts about allowing a datetime.datetime.tzinfo object to be passed instead of a string for tzinfo? That would make testing against a non-local but determined timezone way more reliable than tzoffset.
@charettes You should try my fork! https://github.com/boxed/freezegun/ We freeze time on all tests by default (see also https://medium.com/@boxed/flaky-tests-part-3-freeze-the-world-e4929a0da00e). Our unit test suite took 8 minutes when I introduced this freeze-everything. My fork puts the time back to the ~20 seconds it takes without freezing time at all. Initializing freezegun just once is crucial though since monkey patching back and forth is pretty expensive. That wasn't the biggest performance issue, but it was one we had to fix too.
@boxed out of curiosity, was it taking 8 minutes with 0.3.10 as well?
Seems it takes ~2.2 minutes compared to with my fork 20.2 seconds. So better than 8 for sure, but still pretty bad I'd say. Give my fork a try! And if it works as well for you, give a shout out in my PR :P
I disagree with the premise at the start ofthis ticket.
Really?
My main point is that the faked times returned by Freezegun should not be the same for UTC and time zones with an offset against UTC.
Are you saying basically ignoring the time zone offset (in whatever direction) is a good thing?
Are you saying basically ignoring the time zone offset (in whatever direction) is a good thing?
The system time zone? Absolutely. I would even consider it a critical bug if it did not (which it doesn't currently in a bunch of cases). The point of freezegun is to not respect the system time. Time zones are a part of the system time.
You should not be able to accidentally write tests, when using freezegun, that pass on one machine and then does not pass on another.
If you do want to write tests dealing with time zones other than UTC you should freeze to that time zone. I realize that this is currently rather broken, but we should fix that and not break freezegun more.
I'm fine with not considering the system time zone, yes.
I expect the requested fake date to be reflected both by now() and utcnow() (with the appropriate offset between them).
If you do want to write tests dealing with time zones other than UTC you should freeze to that time zone.
That's what I would do, yes.
I don't know how Freezegun's behavior is influenced by the system time zone, and I think I agree that it shouldn't be.
Regarding the code in my initial post: I have only tested on a single machine with a single (the system) time zone. Maybe the results (in offset between now() and utcnow()) are different with other system time zones (or daylight saving time) or other situations. But the values being equal is definitely wrong in that situation.
Sorry in case my post is a bit confusing. My impression is that we basically agree.