A Date with CPAN, Update #3: Golden Jubilee

[This is an addendum post to a series.  You may want to begin at the beginning.  The last update was update #2.

IMPORTANT NOTE! When I provide you links to code on GitHub, I’m giving you links to particular commits.  This allows me to show you the code as it was at the time the blog post was written and insures that the code references will make sense in the context of this post.  Just be aware that the latest version of the code may be very different.]


In case you missed my talk on Date::Easy from a couple years back, I’ll sum it up for you: dates are hard, y’all.

On January 1st of 2019, a bunch of unit tests for Date::Easy started failing.  It was immediately reported, of course—and can I pause here just a moment to thank Slaven Rezić (SREZIC on CPAN and eserte on GitHub), who is surely the most awesome bug reporter ever?  Date::Easy is definitely a better module for his reports, and I’ve seen him reporting bugs for many others as well.  Anyhow, as I dug into the suddenly failing tests—tests which begin failing even though you didn’t change any code are just a side-effect of writing a date-handling module—I figured out what was wrong.  Happily, it wasn’t a problem with the module, which was still returning correct values, but rather with the unit tests themselves.

One of the challenges you have writing decent unit tests is coming up with a way to test your code.  To take a dumb example, if I’m not sure that my code sub foo { my ($x, $y) = @_; $x * $y } is producing the right answer, I would not write the test thus:
is foo(2, 3), 2 * 3, "multiplication works";
Why not?  Because it doesn’t prove much.  Essentially, that test proves that A = A, which ... well, we knew that already.  I would need to do something a bit crazier, like:
my $product = 0; $product += 2 for 1..3;
is foo(2, 3), $product, "multiplication works";
That’s still pretty dumb, but at least it proves that two independent methods of arriving at an answer give the same result.  And, honestly, sometimes your unit tests have to be dumb like that because you’re trying to avoid doing it the obvious way, because the code does it the obvious way (obviously) and you don’t want to test that A = A. So, as I looked at this code last year, I realized that, while attempting to prove that Date::Easy::Datetime->new(@six_args) produced the proper number of epoch seconds, I was doing something fairly dumb: I was turning the six arguments into a string and then handing them off to str2time, roughly like so:
my $secs = str2time(join(' ', join('/', @args[0,1,2]), join(':', @args[3,4,5])));
Now, as ways to turn 6 numbers into a number of epoch seconds go, that’s a fairly insane one.  Why would I do such a thing?  Well, the obvious way to do it would be to use timegm (or timelocal, as appropriate) ... but that’s what the code was doing.  So I didn’t want to just do that.


Unfortunately, I tripped over a bug in Date::Parse.  This bug has been reported numerous times, although perhaps most concisely in RT/105031.1  The problem had to do with timegm (from Time::Local) handling (or some might say mishandling) the year.  See, when gmtime returns a year as part of its 6-element list, it returns year-minus-1900.  But, when timegm accepts a year, it wonders whether you’re passing a year you got from gmtime, or a year you got from a human.  When you pass it, say, “12,” you probably mean 1912, which is absolutely what it means if you got it from gmtime, because 2012 would be 112.  But, then again, you might have gotten it from a human, who in that case is way more likely to have meant 2012 than 1912.  Of course, if it’s, say, “59” we’re talking about, then whether it is more likely to represent 1959 or 2059 probably depends on when the human who gave it to you gave it to you.  Around the time Linux was invented, 1959 was a scant 10 – 12 years ago and 2059 was a science-fiction future full of flying cars.  Nowadays, 1959 is a time that my children can’t conceive of (one of them asked me once if the world was in black-and-white back then), and, even if someone does manage to invent a flying car that doesn’t explode in the next 30 or 40 years, it seems pretty unlikely that anyone will be able to afford to ride in it.  The solution for the long-ago(-ish) writers2 of Time::Local was the idea of a “rolling century.” If a two-digit year is 50 or more years in the past, it’s more likely to refer to the future.  Sounded sensible at the time, I’m sure.

So we begin to see what went wrong.  One of my sets of arguments to try out was (1969, 12, 31, 23, 59, 59), because it’s the second before the 0-second of (Unix) time, so it makes a nice boundary case, and, when 2019 rolled around, 1969 suddenly became 50 years ago, and timegm decided that “69” meant 2069, so, boom! I hit the Date::Parse bug.  Bummer.

But, wait a tick3 ... didn’t I say that the Date::Easy code was using timegm too?  Why didn’t it have the bug?  Well, I was oversimplifying a bit: while my original code did in fact use timegm (or, again, timelocal, as appropriate), I had already run into that ... well, let’s not call it a bug in Time::Local, but rather an unfortunate design choice ... for my own code and had determined the appropriate fix: just use timegm_modern instead.  What’s that, you say?  Well, the current maintainer of Time::Local (the ever-excellent Dave Rolsky) also knows that the whole “rolling century” can be a pain in the butt, so he offers an alternative: timegm_modern, which does not make assumptions based on what you pass in—whatever you pass in for the year, that’s the year.  If you pass it “69,” it just figures you meant 69A.D. and gives you that.  So I had already figured out that, in almost every case, I wanted to be using that one instead.

Now, the reason I say this is not a bug in Time::Local is that it’s doing exactly what it documents.  Contrariwise, the reason I agree with those who say this is a bug in Date::Parse is because:
[caemlyn:~] perl -MDate::Parse -E 'say scalar localtime str2time("1/1/1969")'
Tue Jan 1 00:00:00 2069
and that’s just crazy talk.  That’s gone beyond assuming something which may or may not be considered silly and actively throwing away information that I provided in order to produce the wrong answer.


In the end, by the way, I solved my 2019 unit test problem by ... switching my unit tests to use timegm_modern/timelocal_modern.  Yes, I’m now testing that A = A, and I’m not happy about that, but at least I’m not blowing up CPAN Testers any more.

Of course, now it’s 2020.  Time for a whole new set of bugs.  As ever, Slaven promptly reported it, and I went looking.  I was immediately suspicious of the very similar nature to last year’s bug set, but this one was in an entirely different unit test.  Still:
#          got: '2070-01-01'
# expected: '1970-01-01'
And it’s now 2020, and 1970 is now exactly 50 years ago?  That can’t be a coincidence.  Sure enough, my unit test contains this code:
my $local_epoch = timelocal gmtime 0;              # for whatever timezone we happen to be in
Uh-oh.  Using timelocal instead of timelocal_modern ... that can’t be good.

Except ... it’s not quite the same problem.  In particular, switching to timelocal_modern doesn’t actually fix the problem.  If you want a pretty detailed-yet-concise description of the issue, Grinnz has you covered, but the short version is, this has nothing to do with being wonky about what two-digits dates mean (or might mean).  This is about being able to reverse what gmtime/localtime spit out.  For a more in-depth discussion of that aspect, Aristotle did a recent post, where he posits that timegm_modern was the wrong solution.  I can’t quite go that far, since it’s really saved my bacon on quite a few other issues, but it certainly is the wrong solution here.

It’s perhaps worthwhile to figure out just WTF I’m doing in the unit test.  See, my first thought was, I want to make sure 0 is considered a valid time (as a number of epoch seconds) instead of being considered an error (since it’s false, at least in Perl’s eyes).  So I figured, I’ll just toss zeroes at various pieces of Date::Easy in various guises and make sure I always get back 1970, which is what Unix considers the beginning of time.  One of those guises is, of course, the actual number 0.  But here I have a problem: 0 is certainly 1/1/1970 ... in England.  In California, where I live, it happens to be 12/31/1969.  So I “solved” that by running 0 through gmtime and then reversing it through timelocal to get what Jan 1 1970 0:0:0 would be in the local timezone.4  Now, one could argue that it kinda defeats the whole purpose, as I’m no longer actually sending zero to the constructor.  But it’s sort of indirectly zero, if you see what I mean, so I let it slide at the time.  Maybe I should just take out the whole thing at this point, now that it’s turned into more trouble than it’s worth.  But I thought to myself, self,5 perhaps it’s worth figuring out how to fix this.

One way to do it would be follow Aristotle’s chain of reasoning, and provide a (third!) interface in Time::Local.6  But I decided to take the opposite tack and created myself a gmtime_sane/localtime_sane which just add the 1900 back to the year before returning.  Right now they only exist in my unit tests’ library module.  But perhaps they deserve some wider attention; I leave it up to you to ponder, dear reader.

I would like to stress, though, that, in both cases, the unit test failures did not indicate any problems with the modules.  Just part of the challenges of making sure Date::Easy stays well-tested as time marches on.

(Interesting side note: the venerable Time::Date module is also suffering from the “Y2020 bug”; you can see a discussion of the—very similar—issue here.)

Currently available on CPAN as 0.09_01 and hopefully graduating to 0.10 soon, CPAN Testers willing.  I hope you folks are getting as much use of it as I am.


The full code for Date::Easy so far is here, and there’s also a new developer’s version.  Of special note:

As always, your feedback (here, in GitHub issues, or in CPAN RT tickets) is always greatly appreciated.



__________

1 Other reports I found included RT/84075, RT/124509, RT/53413, and RT/128158.

2 Or writer ... I actually think the original code was written by Tom Christiannsen, FWIW.

3 Pun very much intended

4 For a refresher on why epoch seconds for dates are always interpreted as local time, even though they’re actually stored as UTC, refer back to part #4 of the series.

5 ‘Cause that’s what I call myself.

6 Well, technically fourth, as there’s also timegm_nocheck/timelocal_nocheck.

6 Comments

I would test multiplication like this:

is foo(2, 3), 6, "multiplication works";

just hard-coding the answer. It sounds like in avoiding hard-coding the answer, you've gotten a fragile test suite that accidentally tests additional modules.

Getting the code right once is enough work...

A variant similar to what you added is coming to a future version of Time::Local. PR#15 (links cause comments to be invisibly held for approval)

>hardcoding test results has other problems

We always coach developers to use hard coded test data to the extent practical. When writing tests you have to unlearn a lot of DRY principles. We tolerate a lot more repetition, and factor it out sparingly.

There are two main reasons why you do this:
1. Any bit of clever calculation in your test could could be wrong. Hard coded values are easy to hand verify.
2. Your test should read like an example of how to use your API, and having lots of layers of indirection and data generators makes the code less readable.
3. When you test inevitably fails, you want a developer to be able to look at the one failing test case in isolation and understand it quickly, without learning your data generation framework.

Yes, you do need to document what the hard coded values mean, and yes you should still use descriptive variable names to help with that.

The article you reference doesn't make a compelling case against this. It seems to be comparing one case where a record is hard coded to another case where the base record is generated elsewhere (action-at-a-distance) but the element of interest hard coded. So 1. it is still hard coding, 2. is making the job of figuring out the actual data being passed to the API harder, and 3. their motivation for doing this is that having more fields populated leads to confusion over which caused the error? That sounds like a failing in the design of the error reporting.


>...due to the effects of timezones and DST...

There are times when you can't avoid creating dynamic data in your tests, but that might not be one of them.

Are you not able to explicitly set environment variable or other flags so that your tests are always executed in a known environment? If you don't control the environment for your test, then you're implemented more of a system test than a unit test.

If your test case isn't about TZ behaviors, then it should force UTC.

Leave a comment

About Buddy Burden

user-pic 12 years in California, 23 years in Perl, 32 years in computers, 53 years in bare feet.