-
Notifications
You must be signed in to change notification settings - Fork 224
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
timezone problem #3
Comments
I can confirm that the test cases you have supplied do in fact fail. I am not certain yet whether or not I think they are valid test cases though. I'll be taking a deeper look into these issues later this week. -John |
The problem is when I freeze or travel, DateTime.now always returns an UTC time (offset is 0) with local values in individual fields (year, hour, etc.). Whereas standard DateTime.now returns a true local time (local values in individual fields and local offset). |
@piglop thanks for following up with more information. This will be helpful. |
I found a solution and made a single commit from master here : http://github.com/piglop/timecop/commit/7478f7ff28f2b5ea4b5476622c5de36bf7edca9f |
@piglop it's taken me a couple of extra days to get around to this. I'll be able to review these patches this weekend and hopefully get them merged in. Thanks again. -John |
Should be fixed with http://github.com/piglop/timecop/commit/607af97d9d352b6b7e077658d2b759704d9749ba |
Included in 0.3.2 release |
I haven't studied the code changes, but timecop 0.3.2 has definitely broken in terms of how Time.now works when freezing. For example, here's a tiny "test" to illustrate:
If I run that with Timecop 0.3.1, I get: Time (local) at start is: Mon Oct 26 13:40:39 -0700 2009 But, if I run that with Timecop 0.3.2, I get: So, now, time has gone backwards. In both cases my local timezone has been changed to UTC. What it's doing in 0.3.2 appears to be that it's adding 3 hours to the "Time.now" result, and not taking into account the Time zone. |
@chris thanks for the report. If you have a free moment and can add a failing test to the test suite, I'd really appreciate it. In the meantime, I suspect you'll get away with 0.3.1. It will probably be several days before I can get around to this. |
While I was trying to write a test case for this problem, I discovered the current test suite fails since DST shift. I couldn't write a test case for chris's problem, but I could reproduce it in a Rails environment. I won't have a lot of time to check these problems this week. |
I'm looking into it. I'm starting to think it's a conflict with Rails though. I forked the code, added a test case and it behaves properly when using simple math to add 3 hours to the time. So, I'm going to look into whether it's Rails' extensions for doing things like "3.hours.from_now" and if that messes with it. Like you guys, I'm pretty busy, and Timecop 0.3.1 works for me, so I'm not sure when I'll resolve this. |
So I've had some time to really dig into this. @chris, I think I have your issues worked out. I'll be releasing a prerelease version of timecop in a couple of days that I'll want you to try out for me. If you care to try my latest release candidate before I get the prerelease version sorted out, you can pull it down from a new branch named 'rewind'. @piglop, there is a separate problem related to the patch you originally submitted. You had noticed behavior where traveling to times/datetimes in a different timezone were losing the timezone information, causing that time to be represented as UTC. Your patch corrected this, but at the same time underscored a new issue that I don't think we can solve. The problem lies in the fact that a DateTime is incapable of representing DST properly. A DateTime only carries an offset (e.g. +0200), but this is not enough information for us to know if this particular DateTime instance should be subject to DST. Different timezones exist within the same offset, and some observe DST while others do not. Due to this limitation, you can end up with the following test case failing:
I have pushed a new branch "rewind" to GitHub where you can see this test currently failing. As a result of this finding, I am seriously considering removing the ability to pass a DateTime instance to Timecop calls. My concern is that this is simply too little known (that DateTime's cannot represent DST) and will cause many people headaches if we allow them to do so. I'm curious what your thoughts are. |
Note that if you had previously installed 0.3.3 that it will take precedence over 0.3.3.rc1. I'm still getting the hang of this prerelease workflow... |
I have released 0.3.4.rc1 as a prerelease gem. You can install it via: gem install/update --prerelease timecop |
Hey guys, I found some other problems with 0.3.4.rc1 that I think I've fixed. Can you take a moment to try upgrading to timecop 0.3.4.rc2 on your respective projects and let me know if I've introduced any regressions? It is a prerelease gem so you'll need to specify that when installing the gem: gem install timecop --pre -v0.3.4.rc2 This is just for verification purposes. Your projects should not depend on this version of the gem. If no regressions are found I will then officially release version 0.3.4 at which point you should upgrade your projects' dependencies. Thanks for your help. -John |
@jtrupiano not sure this is exactly the same bug, but you've fixed mine in your prerelease gem. I have a rails app with the timezone set to Eastern. My box is running Central In 0.3.2: Note it's an hour off. In 0.3.4.rc2 it is correct. |
0.3.4.rc2 is working in all my current tests. It does still change the local time zone to be UTC (from whatever it previously was) though. e.g. (output from my same example above):
|
There are a few issues with time zones.
There's an easy workaround for 2., but 1. is a problem.
I added tests to illustrate this : http://github.com/piglop/timecop/tree/timezone_tests
The text was updated successfully, but these errors were encountered: