(#15496) Correctly determine timezone offset#1330
Merged
zaphod42 merged 4 commits intopuppetlabs:masterfrom Dec 12, 2012
Merged
(#15496) Correctly determine timezone offset#1330zaphod42 merged 4 commits intopuppetlabs:masterfrom
zaphod42 merged 4 commits intopuppetlabs:masterfrom
Conversation
added 2 commits
December 6, 2012 12:19
A user reported the incorrect offset in reports for hosts in the timezone "America/Caracas", which is -04:30 from GMT. The zaml time interpolation did not handle negative fractional timezones correctly, because Ruby rounds down when integer division results in a remainder, which caused "-4.5" to result in "-5". This patch coerces the denominator into a float, so the printf formatting's truncation works correctly.
These exercise the to_zaml monkey-patch to Time by comparing the output of the method, given a couple of strange (fractional offset, both east and west of GMT) time zones, against known-good YAML for the same moment in time -- which happens to be the Mayapocalypse. Without the fix in d2c816a, the test for the condition in #15496 fails with the same incorrect offset described by the bug author: 1) Pure ruby yaml implementation should correctly serialize Time objects, even in oddball timezones Failure/Error: t.should == serialized expected: "--- 2012-12-12 12:12:00.000000 -04:30" got: "--- 2012-12-12 12:12:00.000000 -05:30" (using ==) # ./spec/unit/util/zaml_spec.rb:56 So this seems likely to be testing the right thing.
spec/unit/util/zaml_spec.rb
Outdated
The original tests for testing the Time error were sufficient for that one bug, but we didn't have much around other cases for timezones. This expands the coverage to try to reach more potential edge cases.
The tests previously were fairly disjoint and had many instances of checking that a value round-tripped through YAML correctly. This starts to pull them together and make use of common assertions. It also tries using rspec matchers in order to achieve this. Ideally the matchers would compose, but I don't see how to do that with the rspec interface.
Contributor
|
Thanks for cleaning up the tests! |
zaphod42
added a commit
that referenced
this pull request
Dec 12, 2012
…aml-utc-offset (#15496) Correctly determine timezone offset
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
A user reported the incorrect offset in reports for hosts in the timezone
"America/Caracas", which is -04:30 from GMT. The zaml time interpolation
did not handle negative fractional timezones correctly, because Ruby
rounds down when integer division results in a remainder, which caused
"-4.5" to result in "-5".
This patch coerces the denominator into a float, so the printf
formatting's truncation works correctly.
This pull request closes PR #1323