RFR 8/9: 8068278 ArrayIndexOutOfBoundsException instead of DateTimeException in j.t.chrono.JapaneseChronology.eraOf()

[Lance Andersen]

Hi Roger,

Thank you. Ship it.

Best
Lance
On Feb 3, 2015, at 11:59 AM, Roger Riggs <Roger.Riggs at Oracle.com> wrote:

> Hi Lance,
> 
> Ok, 2nd try.
> 
>    http://cr.openjdk.java.net/~rriggs/webrev-era-8068278/
> 
> 
> 
> On 2/3/2015 11:21 AM, Lance Andersen wrote:
>> Hi Roger,
>> 
>> Did you mean to put the @Test annotation on the class itself?
> It is redundant with the @Test on individual methods but harmless and was there previously.
>> 
>> Also for the test_valueOf test, I would also add the same comment style to it as it is the only one missing a comment
> Not part of this issue, but fixed.
>> 
>> I think you meant to have test_outofrange uses the values from invalidEras array (though I would use a DataProvider myself)  for the values to eraOf as it is currently
> Wow, my bad.
> Replaced with a data provider.  The overhead of DataProviders is a bit higher; but maybe not enough to notice.
> 
> Thanks for reviewing.
> 
> Roger
> 
> 
>> 
>> JapaneseChronology.INSTANCE.eraOf(Integer.MAX_VALUE);
>> 
>> 
>> 
>> Best,
>> Lance
>> On Feb 3, 2015, at 11:10 AM, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>> 
>>> Hi Mandy,
>>> 
>>> I added a test for the invalid Eras.
>>> I do expect the additional conformance tests from the JCK team but this
>>> will synchronize with the fix.
>>> 
>>> Webrev updated in place:
>>>  http://cr.openjdk.java.net/~rriggs/webrev-era-8068278/
>>> 
>>> Thanks, Roger
>>> 
>>> 
>>> 
>>> On 1/30/2015 6:48 PM, Mandy Chung wrote:
>>>> On 1/30/15 2:33 PM, Roger Riggs wrote:
>>>>> Hi Mandy,
>>>>> 
>>>>> Thanks for the review.
>>>>> 
>>>>> I wrote the test (and it passed) but since the JCK folks are providing the tests it seemed
>>>>> undesirable to have duplicate tests.
>>>> 
>>>> JDK developers don't run JCK tests and I think it'd be nice to have a regression test to go with a fix unless the bug is uncovered by an existing test.
>>>> 
>>>> Mandy
>>>>> 
>>>>> Roger
>>>>> 
>>>>> On 1/30/2015 5:27 PM, Mandy Chung wrote:
>>>>>> On 1/30/15 2:25 PM, Roger Riggs wrote:
>>>>>>> Please review this correction of a JapaneseEra range check in java.time.
>>>>>>> The error was discovered during development of additional conformance tests (to be delivered separately).
>>>>>>> 
>>>>>>> Webrev:
>>>>>>>   http://cr.openjdk.java.net/~rriggs//webrev-era-8068278
>>>>>>> 
>>>>>> 
>>>>>> Looks fine to me.  Is it easy to write a regression test to go along with this fix?
>>>>>> 
>>>>>> Mandy
>>>>>> 
>>>>>>> Issue:
>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8068278> 8068278 ArrayIndexOutOfBoundsException instead of DateTimeException in j.t.chrono.JapaneseChronology.eraOf()
>>>>>>> 
>>>>>>> Thanks, Roger
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>> <Mail Attachment.gif>
>> 
>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> Lance.Andersen at oracle.com
>> 
>> 
>> 
> 



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com