Errors in deprecated parse()

[openstrike]

In expanding the test coverage (as suggested in TODO), I have spotted these potential problems with the deprecated parse() method.

1. When parsing from a string, the resulting month is off by one.
2. If no argument is given to parse() it dies gracelessly.
3. If parse is called as a non-method it dies gracelessly.

Given the deprecated status of parse(), how should these be tackled? They could be fixed in parse() or they could be entirely ignored or they could be tested in the test suite and logged there as known problems.

I can make the changes either way but would prefer to do so in the deemed correct manner (whichever that is).

[smith153]

Hi,

Honestly, I won't touch parse(). I don't think it is documented any where (?). I don't actually know what it is for. Looking back in releases for 10+ years ago, parse was always there, but it was never used by Time::Piece itself or in the docs. I can only assume it was intended as a poor man's strptime(). I think early on (in the early 2000's), there was poor strptime support on many OS's of that era. I'm guessing someone tried to compromise and write their own parse function. But in my opinion, we should just use strptime.

[smith153]

If you really want to write tests, the main problem with Time::Piece right now is the strptime function. Because of poor support in OS's from 10+ years ago, a custom strptime function was copied and pasted into Piece.xs. This was a good compromise, but there were a few issues. Mainly the interface was not fully implemented, and it can only parse english date names (there is a hardcoded array of date names in the source code).

I thought about rewriting it (strptime) in pure perl, but now I feel I would just add more bugs, especially since I've seen bug reports filed for other date modules in other languages where people were trying to parse dates like "January 1st, 1750".

Nonetheless, I think a good test case for strptime would be "can it parse what is outputted by strftime?". In this case, start with a known set of dates and output them in different formats via strftime. Then using the same format string, ensure that strptime can parse them back into an object, call strftime on that object and compare its output with the original string that was feed into strptime. They should always be the same and should always work. I think there are already some examples at the bottom of t/02core.t, but they use simple dates, with no date names or timezones. In our current case it would only work on OS's where english is the configured language (due to our use of hardcoded date names in Piece.xs), but that needs to be fixed. Perhaps here soon I'll restore the call to a native strptime and bypass the one in Piece.xs (much like how we handle strftime now). So since most of the sore spots in Time::Piece are around strptime, I feel plenty of tests around that would be a good thing. But feel free to do what ever you'd like test-wise.

[openstrike]

Re: parse(), I fully agree that it makes no sense to work on it if nobody is using it and it is officially deprecated. So let's just leave it bugs and all. I'll include the tests for it which will pass because of the bugs. The tests can then be removed if and when parse() is finally taken from the codebase.

strptime did seem like a good candidate for testing but then I read about the idea of replacing it with a pure perl version and didn't want to put the cart before the horse. If that PP version is now on hold (or binned) then I'll look to implement what you suggest and submit issues for any bugs which show up.

In the meantime I'll commit what I have done so far on the coverage which is to call the remaining subs and go through some of the less obvious conditions and branches.

[smith153]

Indeed, but before I was going to touch strptime, I was going to write a few tests around it. Its always good to know whats currently broken and what gets fixed :)

[openstrike]

The extra tests for general coverage are now on the coverage branch in my repo at c9317ea. All subs are now tested. If you see anything there requiring changes just let me know.

I'll now start on your suggestions for testing strptime.