fix Date.strptime monkeypatch #217
Conversation
|
One of the dependencies doesn't seem to support ruby 1.9.3 anymore. Thoughts on removing support for 1.9 in |
|
I'd vote to remove it (and bump the gem minor version). It can't support old rubies forever! |
| def test_date_strptime_with_invalid_date | ||
| begin | ||
| Date.strptime('', '%Y-%m-%d') | ||
| rescue ArgumentError => e |
There was a problem hiding this comment.
Better to use assert_raises: https://apidock.com/ruby/Test/Unit/Assertions/assert_raise
| @@ -48,7 +48,17 @@ def strptime_with_mock_date(str = '-4712-01-01', fmt = '%F', start = Date::ITALY | |||
| "supports Date::ITALY for the start argument." | |||
| end | |||
|
|
|||
| Time.strptime(str, fmt).to_date | |||
| d = Date._strptime(str, fmt) || Date.strptime_without_mock_date(str, fmt) | |||
There was a problem hiding this comment.
Can you please explain why is there are _strptime or strptime_without_mock_date, and not just strptime_without_mock_date used?
There was a problem hiding this comment.
Based on the docs https://ruby-doc.org/stdlib-2.6/libdoc/date/rdoc/Date.html#method-c-_strptime returns a hash instead of a date object. I think I was thinking I didn't need the actual date object, but I wrote that too long ago to remember.
It looks like strptime actually calls _strptime internally, so we could probably just go with strptime_without_mock_date. Do the tests still pass that way?
cc #242 <--- seems better to merge
|
Merged the equivalent in #242 based on this PR. Thanks! |
See #108, #115, #116, #132.