Fixed tests to_time returning an instance of Time in the local system timezone
#9653
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carlosantoniodasilva
added a commit
that referenced
this pull request
Mar 10, 2013
Fixed tests `to_time` returning an instance of `Time` in the local system timezone. Tests for to_time returning an instance of Time in the local system timezone did not take in account DST when they used mixed timezone declarations like 'US/Eastern' and '-0500'. Replaced timezone with DST by timezone without DST.
|
Turns out that this was actually a bug (#10306). Moscow didn't stop using DST until March 27, 2011 when the clocks went forward an hour and didn't go back in the autumn so the times in the tests are +0300 and not +0400. |
claudiob
added a commit
to claudiob/rails
that referenced
this pull request
Oct 30, 2014
Problem ------- The [following test](https://github.com/rails/rails/blob/d20f7b043a537b57ff4a7911f65de2fb7b7aea7d/activesupport/test/core_ext/string_ext_test.rb#L411) fails on my machine ```ruby StringConversionsTest#test_partial_string_to_time [test/core_ext/string_ext_test.rb:411]: Expected: 2014-10-30 18:50:00 +0300 Actual: 2014-10-30 17:50:00 +0300 ``` Cause ----- In the United States, Daylight Saving Time ends on the first Sunday of November (see [Wikipedia article](http://en.wikipedia.org/wiki/Daylight_saving_time_by_country#Daylight_Saving_Time_rules_as_of_2014)). In Russia, Daylight Saving Time is… undergoing changes (as pointed out in rails#17395 and rails#17409). Since I am running the tests from a machine located in the United States (PST), the time difference expected by the test does not always hold true. The same test might be passing when run from a machine in a different time zone. The timezone for the tests was set to "Europe/Moscow" in rails#9653 by @ptfg with the idea of fixing a test. However, @pixeltrix [later found out](rails#9653 (comment)) that using "US/Eastern" was okay; the error was caused by the fact that Moscow didn't stop using DST until March 27, 2011. Solution -------- I think it's safe to set the time zone of the tests back to "US/Eastern", since "Europe/Moscow" is a timezone that has recently been changing and running the tests against that time zone might pass on same machines and fail on other ones. Thanks to @matthewd for the suggestion. How to reproduce ---------------- 1. Temporarily change the date of your machine to October 30th, 2014 (to ensure DST has not ended) 2. Set the time zone of your machine to PST 3. Run `ruby -Itest test/core_ext/string_ext_test.rb`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tests for
to_timereturning an instance ofTimein the local system timezone did not take in account DST when they used mixed timezone declarations like 'US/Eastern' and '-0500'.Replaced timezone with DST by timezone without DST.