RFR(XS): 8169003: LogDecorations.iso8601_utctime_test fails if numeric locale uses ", " as separator between integer and fraction part
marcus.larsson at oracle.com
Tue Nov 22 12:32:21 UTC 2016
On 2016-11-21 17:38, Kirill Zhaldybin wrote:
> Thank you for reviewing the fix!
>> ISO8601 says the decimal point can be either '.' or ',' so the test
>> should accept either. You could let sscanf read out the decimal point
>> as a character and just verify that it is one of the two.
>> In the UTC test you changed ASSERT_GT to ASSERT_EQ, which means that
>> we won't accept "Z" suffixed strings. Please revert that.
> I agree that ISO8601 could add "Z" to time (and as far as I understand
> date/time without delimiters is legal too) but these are the unit tests.
> Hence they cover the existing code and they should pass only if the
> result corresponds to existing code and fail otherwise.
> The current code from os::iso8601_time format date/time string
> %04d-%02d-%02dT%02d:%02d:%02d.%03d%c%02d%02d so we should not consider
> any other format as valid.
> Could you please let me know your opinion?
I think the test should verify the intended behavior, not the
implementation. If we refactor or change something in iso8601_time() we
shouldn't be failing the test if it still conforms to ISO8601, IMO.
> Thank you.
> Regards, Kirill
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8169003
>>> Thank you.
>>> Regards, Kirill
More information about the hotspot-dev