You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Jim Yu (JIRA)" <ji...@apache.org> on 2009/05/12 18:23:45 UTC
[jira] Created: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
[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);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Created: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by Jim Yu <ju...@gmail.com>.
I see. Thanks, Nathan.
2009/5/16 Nathan Beyer <nd...@apache.org>
> On Fri, May 15, 2009 at 12:11 PM, Jim Yu <ju...@gmail.com> wrote:
> > 2009/5/15 Oliver Deakin <ol...@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 <ol...@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
> >
>
--
Best Regards,
Jim, Jun Jie Yu
Re: [jira] Created: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by Nathan Beyer <nd...@apache.org>.
On Fri, May 15, 2009 at 12:11 PM, Jim Yu <ju...@gmail.com> wrote:
> 2009/5/15 Oliver Deakin <ol...@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 <ol...@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
>
Re: [jira] Created: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by Jim Yu <ju...@gmail.com>.
2009/5/15 Oliver Deakin <ol...@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 <ol...@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
Re: [jira] Created: (HARMONY-6207) [classlib][text] SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by Oliver Deakin <ol...@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 <ol...@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
>
>> 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
Re: [jira] Created: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by Jim Yu <ju...@gmail.com>.
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 <ol...@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.
> 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.
>
>
> 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
>
>
--
Best Regards,
Jim, Jun Jie Yu
Re: [jira] Created: (HARMONY-6207) [classlib][text] SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by Oliver Deakin <ol...@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 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? :)
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
[jira] Updated: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tim Ellison updated HARMONY-6207:
---------------------------------
Fix Version/s: (was: 5.0M10)
> [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
> Attachments: HARMONY-6207.diff
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by "Jim Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jim Yu updated HARMONY-6207:
----------------------------
Attachment: (was: HARMONY-6207.diff)
> [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
>
> Attachments: HARMONY-6207.diff
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by "Jim Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jim Yu updated HARMONY-6207:
----------------------------
Attachment: HARMONY-6207.diff
> [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
>
> Attachments: HARMONY-6207.diff
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by "Jim Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jim Yu updated HARMONY-6207:
----------------------------
Attachment: HARMONY-6207.diff
> [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
>
> Attachments: HARMONY-6207.diff
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12720063#action_12720063 ]
Tim Ellison commented on HARMONY-6207:
--------------------------------------
Jim,
Please can you explain the patch? What is the purpose of using the Date constructors with numerical arguments etc. It unclear to me what this is doing.
Thanks.
> [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
> Attachments: HARMONY-6207.diff
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-6207) [classlib][text]
SimpleDateFormatTest.test_parseLjava_lang_StringLjava_text_ParsePosition
would fail
Posted by "Jim Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12720075#action_12720075 ]
Jim Yu commented on HARMONY-6207:
---------------------------------
Hi Tim,
Our implementation of SimpleDateFormat class is correct 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.) 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. So I think we can create Date instances directly to generate the expected values.
BTW, the non-bug behavior difference between GregorianCalendar instances of Harmony and ICU is another issue that we might need to consider how to solve it. But I don't think it is the blocker for this case.
> [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
> Attachments: HARMONY-6207.diff
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> 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);
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.