Skip to content
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

Mojo::Date: epoch can be a negative number #1079

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

aferreira
Copy link
Contributor

Summary

Fixes Mojo::Date when used with negative numbers as epoch

Motivation

I found the issue when representing birth dates with Mango::Time objects - a date before the epoch like 1965-12-21 would emit uninit warnings and end up represented as Jan 1, 1970. This comes from https://github.com/oliwer/mango/blob/master/lib/Mango/BSON/Time.pm#L12 which in turn points to Mojo::Date code https://github.com/kraih/mojo/blob/master/lib/Mojo/Date.pm#L25

For example, running the test included in this PR with the code in master gives the output

$ prove -I lib t/mojo/date.t
t/mojo/date.t .. 1/? Use of uninitialized value in gmtime at /Users/ferreira/ws/mojo/lib/Mojo/Date.pm line 66.

#   Failed test 'right format'
#   at t/mojo/date.t line 81.
#          got: 'Thu, 01 Jan 1970 00:00:00 GMT'
#     expected: 'Tue, 21 Dec 1965 00:00:00 GMT'
# Looks like you failed 1 test of 40.
t/mojo/date.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/40 subtests 

Test Summary Report
-------------------
t/mojo/date.t (Wstat: 256 Tests: 40 Failed: 1)
  Failed test:  33
  Non-zero exit status: 1
Files=1, Tests=40,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.02 cusr  0.00 csys =  0.05 CPU)
Result: FAIL

References

None

@marcusramberg
Copy link
Member

Seems like a good fix with a reasonable test.

@jhthorsen
Copy link
Member

I didn't know negative epoch was a thing, but the actual fix seems reasonable to me as well.

@kraih
Copy link
Member

kraih commented Apr 4, 2017

Thank you, looks like this passed the vote.

@kraih kraih closed this Apr 4, 2017
@kraih kraih reopened this Apr 4, 2017
@kraih kraih merged commit d313320 into mojolicious:master Apr 4, 2017
@kraih
Copy link
Member

kraih commented Apr 4, 2017

Afraid i had to revert this change again, since once i added the missing tests for fractional negative epoch values it wasn't portable anymore. d87581f

@kraih
Copy link
Member

kraih commented Apr 4, 2017

Failing test can be seen here. https://travis-ci.org/kraih/mojo/jobs/218440126

@kraih
Copy link
Member

kraih commented Apr 4, 2017

GitHub won't let me reopen this pull request, please open a new one if you find a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants