harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Deakin <oliver.dea...@googlemail.com>
Subject Re: [jira] Created: (HARMONY-6207) [classlib][text] SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition would fail
Date Fri, 15 May 2009 15:52:26 GMT
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.

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.


>> 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

View raw message