harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nathan Beyer <ndbe...@apache.org>
Subject Re: [jira] Created: (HARMONY-6207) [classlib][text] SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition would fail
Date Sat, 16 May 2009 01:42:16 GMT
On Fri, May 15, 2009 at 12:11 PM, Jim Yu <junjie0122@gmail.com> wrote:
> 2009/5/15 Oliver Deakin <oliver.deakin@googlemail.com>
>
>> Jim Yu wrote:
>>
>>> Hi Oli,
>>>
>>> Thanks for your attention on this issue. My explanation as below. I hope
>>> it
>>> could help explain the reason:-)
>>>
>>> 2009/5/15 Oliver Deakin <oliver.deakin@googlemail.com>
>>>
>>>
>>>
>>>> Hi Jim,
>>>>
>>>> I just had a look at this JIRA and had a question about it. It sounds
>>>> from
>>>> your description that we are seeing a difference between ICU's and
>>>> Harmony's
>>>> implementation of some Calendar related classes. This seems to me like a
>>>> bug
>>>> in either our code or ICU's and I wasn't sure that fixing the tests to
>>>> pass
>>>> was the right thing to do
>>>>
>>>>
>>>
>>>
>>> The failing testcase is for SimpleDateFormat class but there is no bug of
>>> this class for this testcase. The reason of why the testcase failed is
>>> that
>>> we used getTime method of GregorianCalendar instance to create a Date
>>> instance as the expected value. However, there is a non-bug behavior
>>> difference between GregorianCalendar instances of Harmony and ICU.(ICU
>>> complies with newer version of CLDR while Harmony complies with older one
>>> I
>>> guess. Please correct me if there is something incorrect) So when the
>>> testcase want to assert that the expected Date instance created by Harmony
>>> equals the result Date instance created by ICU is true, it failed. In a
>>> word, the testcase discovered a non-bug difference of GregorianCalendar
>>> between Harmony and ICU other than a bug of SimpleDateFormat class.
>>>
>>>
>>
>> Ahh ok, that makes more sense now.
>>
>>
>>>
>>>> here. My thoughts were that we need to do one of:
>>>> - raise a bug with ICU and fix the tests for now.
>>>> - fix a bug in our GregorianCalendar class.
>>>> - delegate from our GregorianCalendar class to ICU's version (why don't
>>>> we
>>>> do this already?).
>>>>
>>>> I may be missing something, so I thought I'd ask - it just seems that
>>>> there
>>>> is a bug/discrepancy here somewhere that needs to be fixed. I guess my
>>>> question is: why is this a fix to the tests and not the code? :)
>>>>
>>>>
>>>
>>>
>>> Our implementation of SimpleDateFormat class is correct for this testcase,
>>> so we don't need to make a fix to the code. BTW, to delegate from our
>>> GregorianCalendar class to ICU's version is a work we need to do. (Maybe
>>> there is a lot work to do as we need to delegate Calendar class as well
>>> and
>>> might need to update the testcase for them) But in this case, it is not
>>> necessary as we can use Date instance directly to create expected value
>>> other than via creating a GregorianCalendar instance first and calling
>>> getTime after that.
>>>
>>>
>>>
>>
>> Ok, so we can work around this difference for now. What do you think is the
>> right thing to do for the future? Should we delegate Calendar
>> responsibilities to icu?  I can see upsides and downsides - upsides are that
>> we no longer need to maintain the code for the Calendar functions, downsides
>> are that there may be some overheads of delegating through to icu which may
>> cause performance degradation, and we will also need to test which
>> implementation gives us the better performance overall.
>>
>
> Currently, we have several classes which have already delegated the
> implementation to ICU. E.g.  format relevant classes in text module and
> timezone relevant classes in luni module. However, there are some classes
> which can be delegated to icu but not done yet. E.g. Calendar relevant
> classes. I'm not sure what the real reason is. Maybe we have investigated
> the downsides for them and decided to leave them as they are. Or we have not
> investigated the issue yet. If the fact is the later one, I will investigate
> this issue after moving the remaining tests of text module out of the
> exclude list(have moved some out, will keep working).

It's the later - it hasn't been investigated.

>
>>
>> Perhaps for now it is easier to leave the code as it is and address the
>> issues above when we find a user/app that is affected by them.
>>
>
> Agree.  I think there might be other classes which can be considered to
> delegate to ICU. We can address them first and do the actual work when we
> find something are blocked by them.
>
>>
>> Regards,
>> Oliver
>>
>>
>>
>>>
>>>> Regards,
>>>> Oliver
>>>>
>>>>
>>>> Jim Yu (JIRA) wrote:
>>>>
>>>>
>>>>
>>>>> [classlib][text]
>>>>> SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
>>>>> would fail
>>>>>
>>>>>
>>>>> ----------------------------------------------------------------------------------------------------
>>>>>
>>>>>                Key: HARMONY-6207
>>>>>                URL: https://issues.apache.org/jira/browse/HARMONY-6207
>>>>>            Project: Harmony
>>>>>         Issue Type: Test
>>>>>         Components: Classlib
>>>>>   Affects Versions: 5.0M9
>>>>>           Reporter: Jim Yu
>>>>>            Fix For: 5.0M10
>>>>>
>>>>>
>>>>> Currently, the testcase
>>>>> SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
>>>>> would fail. I've investigated the root cause of this failure and found
>>>>> the
>>>>> main reason is that the GregorianCalendar class used in the testcase
is
>>>>> implemented by Harmony itself not delegating to ICU. So when we call
>>>>> getTime
>>>>> of GregorianCalendar to get an Date instance as the expected value, it
>>>>> would
>>>>> not equal to the one created by ICU as the result. E.g. the following
>>>>> testcase [1] would fail while [2] can pass. So I use Date instances
>>>>> directly
>>>>> for these failing ones in my patch. [1] test.parse("yyy", "99", new
>>>>> GregorianCalendar(99, Calendar.JANUARY, 1)
>>>>>               .getTime(), 0, 2);
>>>>> [2] test.parse("yyy", "99", new com.ibm.icu.util.GregorianCalendar(99,
>>>>> Calendar.JANUARY, 1)
>>>>>               .getTime(), 0, 2);
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>> --
>>>> Oliver Deakin
>>>> Unless stated otherwise above:
>>>> IBM United Kingdom Limited - Registered in England and Wales with number
>>>> 741598. Registered office: PO Box 41, North Harbour, Portsmouth,
>>>> Hampshire
>>>> PO6 3AU
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>> --
>> Oliver Deakin
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with number
>> 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
>> PO6 3AU
>>
>>
>
>
> --
> Best Regards,
> Jim, Jun Jie Yu
>

Mime
View raw message