Date arithmetic can be dangerous
Every time I review code others have written, I blame people for doing date arithmetic of their own. However, some time ago, I received a pull request for a module that had some date arithmetic inside. As all tests passed, I could not see something dangerous in it and followed the pull request. Today, I found the date tests failing. Why? Why today? Well, this is worth some investigation.
The main part of the module generates an HTTP-Header using this construct ($c is the mocked catalyst context, expire_in is a method containing the nr of seconds to expire in):
$c->response->headers->expires(time() + $self->expire_in)
if ....some_condition...
Well, adding a number of seconds to an epoch value cannot hurt. Can it? The test looked like this:
my $expected_date =
(DateTime->now
+ DateTime::Duration->new(seconds => $controller->{expire_in}
)->strftime('%a, %d %b %Y %H:%M:%S GMT');
is $c->response->header('expires'), $expected_date,
'expired header date is OK';
For the test case having an expiry of 3 years, the test failed with a difference of exactly one second:
# Failed test 'expired header date is OK' # at t/5-expire.t line 55. # got: 'Thu, 19 Feb 2015 18:27:59 GMT' # expected: 'Thu, 19 Feb 2015 18:27:58 GMT' not ok 4 - expired header date is OK
One second? Do we have a rounding problem? No. After a while, I remembered that DateTime mentioned a leap-second in its Change-Log some time ago. This could explain our difference.
Doing time-calculations by ourselves is wrong. Replacing the header generation with a more complicated but correct construct worked. Using DateTime before 0.71 kept the old date arithmetic-tests working, but actually the tests should have failed, because the leap-second was ignored.
$c->response->headers->expires(
DateTime->now
->add(seconds => $self->expire_in)
->epoch
)
My personal conclusion: Date arithmetic is dangerous. Be more strict to any kind of date arithmetic. Never ever do date-math again. Never. Instead: keep all date manipulating modules at a current version and trust in them. Their authors known what they are doing, I don't. I didn't :-(
I blog about Perl.
Just because you doesn't know how date arithmetic is done, doesn't meen it's dangerous! Assuming that a day consist of 60*60*24 seconds is ignorant!
Maybe my acceptance of a pull request without decent knowledge of date arithmetic was wrong. Let me assume that most of us do not know every bit of every Topic they are involved. This is exactly the point where modules come into play. Using components that are well tested are even better than trying to reinvent the wheel. This is my lesson I learned from this incident.
I once had a similar problem, then partly due to daylight savings times. It's an interesting question: should tests of date/time be based on the module testers environment, or on a specific environment established by the module author? My preferred solution was to avoid DateTime->now in tests, because it means the tests are now dependent on whatever date/time context is in force when the tests are run. I used a chosen specific time for most tests, because that allowed my tests to specifically check particular leap intervals and timezone switches.
Doing date/time arithmetic yourself [properly] is a lot of work. Leap-seconds, in the grand scheme of things, are relativity easy to deal with but daylight savings changes are a real nightmare. Handling them properly requires keeping track of the entire history of changes of every geographic locale.
For the latest tests concerning dates and times I have written, I used this pattern:
my $dt = DateTime->new( some date ); no warnings 'redefine'; local *DateTime::now = sub { $dt->clone };# run the test and compare against $dt
If the module under test also used DateTime, I had predictable values for testing. Of course, edge cases and special conditions are depending on the value injected as 'now' (as morungos already mentioned). I don't know if this is the best way to do it but at least I have the feeling not to get surprises...
And if bugs arise, adding new test cases then is very simple.