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

[Roger Riggs]

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 
> <mailto: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/ 
>> <http://cr.openjdk.java.net/%7Erriggs/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 
>>>>>> <http://cr.openjdk.java.net/%7Erriggs//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
>>>>>>
>>>>>
>>>>
>>>
>>
>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.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 <mailto:Lance.Andersen at oracle.com>
>
>
>