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.ri...@oracle.com
<mailto:roger.ri...@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
