Faked time shouldn't always be the same. about freezegun HOT 43 OPEN

@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 moving
• freeze_time('2001-01-02', tzinfo='local') for respecting local time zone but locking the rest
• freeze_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.

from freezegun.

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?

from freezegun.

This ticket is also in direct conflict with #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 .

from freezegun.

So how are we supposed to test code that travels in time in a certain timezone which is not UTC (and has DST)?

Supposing I have a function that returns the Unix timestamp of the beginning of the sixth month from now (1st, 00:00:00), localtime. How would I test it? More specifically, how would I test that it actually takes localtime into account?

from freezegun.

From this thread we can get some test cases on how it should work, and from the other duplicates as well. I guess no one actually did a full summary of the related issues?

I guess I can give this a shot as I need to work around this right now.

from freezegun.

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.

from freezegun.

@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.

from freezegun.

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.

from freezegun.

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.

Can't agree more!

from freezegun.

Oh, and Freezegun's fake date/time lacks milliseconds compared to the default update. I've tested on Python 3.4.2.

from freezegun.

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_offset is passed?

I'm also quite worried about how the documentation will read.

from freezegun.

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.

from freezegun.

I agree it should be the local time. Caller can specify UTC as the timezone if they want that.

from freezegun.

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.

from freezegun.

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.

1. 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.
2. 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 use utcnow() in code) it seems simpler to document a workaround that says "add -00:00 to your freeze argument".
3. 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() and utcnow().

Whatcha think @spulec?

from freezegun.

@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.

from freezegun.

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. :-)

from freezegun.

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.

from freezegun.

@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 github-linguist/linguist#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

from freezegun.

Woah @homeworkprod - the more you know!

from freezegun.

@bryanhelmig, you're welcome. :)

from freezegun.

Is this project still maintained? It would be good to get something merged for this.

from freezegun.

Yes, open to a PR with a clean merge and tests passing.

from freezegun.

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.

from freezegun.

@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.

from freezegun.

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().

from freezegun.

I've been trying to make a PR to fix some issues from @azmeuk . It seems all of #205
#204
#209
#208
#240

and this ticket are basically the same underlying issue. The tickets should be probably be merged.

from freezegun.

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.

from freezegun.

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 #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.

from freezegun.

@boxed out of curiosity, was it taking 8 minutes with 0.3.10 as well?

from freezegun.

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

from freezegun.

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?

from freezegun.

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.

from freezegun.

I expect the requested fake date to be reflected both by now() and utcnow() (with the appropriate offset between them).

But what would the appropriate offset be if a timezone is not specified? To me it seems there are only two alternatives when you haven't specified a time zone: UTC or -13 (a time zone that is uninhabited!). Currently it's UTC (or it's broken, but let's ignore that for now).

Sorry in case my post is a bit confusing. My impression is that we basically agree.

It seems so. That's pretty good news! :P

from freezegun.

UTC seems like the single reasonable default to me to avoid unexpected results on different systems.

Requiring the user to specify a zone is probably a bit too much.

from freezegun.

Good. Then you now think that the example you started this issue with is in fact what you would expect?

from freezegun.

Yeah, it starts to sink in.

Let me try this change: Given I would have explicitly specified Europe/Berlin as the zone as part of my test/Freezegun setup (rather expecting that zone to be considered because it's the system default), then I would expect the results I've described in my 3.5 years old initial post.

from freezegun.

Cool. Then we are all in agreement what should happen at least.

from freezegun.

with freeze_time(datetime.datetime(year=2020, month=1, day=1,
                                       microsecond=400000),
                     tz_offset=4) as frozen:
        dt1 = datetime.datetime.now(tz=datetime.timezone.utc)
        dt2 = datetime.datetime.utcnow().replace(tzinfo=datetime.timezone.utc)
        assert dt1 == dt2

I assume the above assertion should hold, but it does not. Considering how long the original issue has been open a quick resolution does not seem likely, but could the now() take the tz given as argument into account somehow? Is this discussion ongoing in some other venues?

EDIT:

I get the following error running the above.

Expected :FakeDatetime(2020, 1, 1, 0, 0, 0, 400000, tzinfo=datetime.timezone.utc)
Actual   :FakeDatetime(2020, 1, 1, 4, 0, 0, 400000, tzinfo=datetime.timezone.utc)
<Click to see difference>

def test_freezegun():
        with freeze_time(datetime.datetime(year=2020, month=1, day=1,
                                           microsecond=400000),
                         tz_offset=4) as frozen:
            dt1 = datetime.datetime.now(tz=datetime.timezone.utc)
            dt2 = datetime.datetime.utcnow().replace(tzinfo=datetime.timezone.utc)
>           assert dt1 == dt2
E           assert FakeDatetime(2020, 1, 1, 4, 0, 0, 400000, tzinfo=datetime.timezone.utc) == FakeDatetime(2020, 1, 1, 0, 0, 0, 400000, tzinfo=datetime.timezone.utc)

tests/test_logging.py:25: AssertionError

from freezegun.

@sjlehtin The problem is that no one has implemented what we decided. We do welcome PRs though if you want to give it a shot.

from freezegun.

I've been experimenting a bit, and I'm a little bit confused about the idea of test case test_datetime_timezone_real_with_offset:

def test_datetime_timezone_real():
    now = datetime.datetime.now(tz=GMT5())
    assert now == datetime.datetime(2012, 1, 14, 7, tzinfo=GMT5())
    assert now.utcoffset() == timedelta(0, 60 * 60 * 5)


@freeze_time("2012-01-14 2:00:00", tz_offset=-4)
def test_datetime_timezone_real_with_offset():
    now = datetime.datetime.now(tz=GMT5())
    assert now == datetime.datetime(2012, 1, 14, 3, tzinfo=GMT5())
    assert now.utcoffset() == timedelta(0, 60 * 60 * 5)

Why should the latter, given that datetime.datetime.now() is passed real tz object, care about the tz_offset?

Elsewhere, freeze_time is treated as UTC and tz_offset the offset from that (UTC). Now, in this case, tz_offset and the given TZ are combined to arrive at a different value for the UTC time.

from freezegun.

I think I'm hitting the gist of @homeworkprod and @boxed have been talking in this thread. What was the earlier decision and did you have some test cases or examples to make those intuitions concrete?

from freezegun.

Here is a doodle sjlehtin@8237820, look like something worth taking forward? @boxed

from freezegun.