On Fri, May 15, 2009 at 12:11 PM, Jim Yu <[email protected]> wrote: > 2009/5/15 Oliver Deakin <[email protected]> > >> 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 <[email protected]> >>> >>> >>> >>>> 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 >
