[Libosinfo] [osinfo-db PATCH] tests, test_dates: Keep just one implementation of _parse_iso_date()

[Cole Robinson]

On 5/22/19 9:22 AM, Pino Toscano wrote:
> On Wednesday, 22 May 2019 15:16:27 CEST Fabiano Fidêncio wrote:
>> As suggested by Cole, let's keep just one implementation of
>> _parse_iso_date() (the most compatible one) and avoid diverging over
>> time.
>>
>> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
>> ---
>>  tests/test_dates.py | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/test_dates.py b/tests/test_dates.py
>> index 49df613..aca7472 100644
>> --- a/tests/test_dates.py
>> +++ b/tests/test_dates.py
>> @@ -8,14 +8,10 @@ import sys
>>  from . import util
>>  
>>  
>> -if sys.version_info >= (3, 7):
>> -    def _parse_iso_date(date_string):
>> -        return datetime.date.fromisoformat(date_string)
>> -else:
>> -    def _parse_iso_date(date_string):
>> -        m = re.match("([0-9]{4})-([0-9]{2})-([0-9]{2})", date_string)
>> -        assert m
>> -        return datetime.date(int(m.group(1)), int(m.group(2)), int(m.group(3)))
>> +def _parse_iso_date(date_string):
>> +    m = re.match("([0-9]{4})-([0-9]{2})-([0-9]{2})", date_string)
>> +    assert m
>> +    return datetime.date(int(m.group(1)), int(m.group(2)), int(m.group(3)))
> 
> The purposes of having the two implementation are:
> - datetime.date.fromisoformat() is available in newer versions, so it
>   makes sense to leverage it

What are the concrete benefits we receive from using the library call,
when we still need to maintain the old code path? All the code reuse
benefits are pretty much thrown out if we need to maintain the old code path

> - my implementation is not optimized as datetime.date.fromisoformat();
>   considering this is only a test, it is "good enough"
> - having the version check means it is easier to drop the old
>   implementation when raising the minimum Python version required
> 

Considering we will want to be compatible with running tests on RHEL8
which has python3.6 by default, we are talking a long time before we can
depend on python3.7+. In the intervening years, if a dev wants or needs
to change this code, we would need to test against two python versions
which is a pain, or push it off to CI to catch. It adds complexity for
hypothetical benefits. Doesn't seem worth it IMO.

But if you don't find that convincing, I won't argue it further :)

Thanks,
Cole