harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Yu <junjie0...@gmail.com>
Subject Re: [jira] Created: (HARMONY-6207) [classlib][text] SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition would fail
Date Fri, 15 May 2009 17:11:09 GMT
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).

>
> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message