Re: RFR 8235699 : ArrayIndexOutOfBoundsException in CalendarBuilder.toString

[Roger Riggs]

Hi Clive,

The update looks good to me.

Thanks, Roger


On 2/4/20 5:17 PM, Verghese, Clive wrote:

Hi Roger,

Thank you for the feedback. I have addressed your comments and updated the 
Webrev.
http://cr.openjdk.java.net/~alvdavi/webrevs/8235699/

Regards,
Clive Verghese


On 2/4/20, 12:34 PM, "core-libs-dev on behalf of Roger Riggs" 
<core-libs-dev-boun...@openjdk.java.net on behalf of roger.ri...@oracle.com> wrote:

     Hi Clive,

A few comments: The copyrights for the new files need to follow the template in

     <repo>make/template/gpl-header.
     In particular, it needs to include Oracle as a copyright holder.

bug235699.java: The @summary should be more informative; "it works" tells nothing about

     the case being tested.

Please rename bug8235699.java to Bug8235699.java;

     There are more initial upper case tests than lower and it would be good
     to converge over time.

35: A comment saying that a AIOOBE should not occur would be helpful. CalendarBuilderTest.java: Should describe the condition that is being created.

     27:  "Test that CalendarBuilder.toString does not produce IOOBE"

Are all of the assignments necessary to cause the bug?

     Remove any that are not; they are misleading.  (onlysetting the year is
     needed)

Thanks, Roger On 1/2/20 4:18 PM, Verghese, Clive wrote:

     > Hi Alan,
     >
     > Thanks for the feedback,
     >
     > I have removed the @Author tag and updated the tests as per your 
recommendation.
     >
     > Updated Webrev
     > http://cr.openjdk.java.net/~phh/8235699/webrev.04/
     >
     > Regards,
     > Clive Verghsese
     >
     > Regards,
     > Clive Verghese
     >
     > On 1/2/20, 11:19 AM, "Volker Simonis" <simon...@amazon.de> wrote:
     >
     >      On 02.01.20 18:39, Alan Bateman wrote:
     >      > On 02/01/2020 13:26, Volker Simonis wrote:
     >      >> :
     >      >> http://cr.openjdk.java.net/~simonis/webrevs/2020/8235699.02/
     >      >>
     >      >> Ready to push?
     >      >>
     >      > You shouldn't need to use core reflection here. Instead you can 
create
     >      > the test so that it is compiled and runs as if part of the 
java.text
     >      > package, e.g.
     >      >
     >      > @build java.base/java.text.CalendarBuilderToStringTest
     >      > @main Driver
     >      >
     >
     >      Thanks for the hint. I wasn't aware of this possibility.
     >      I think Clive will rewrite the test.
     >
     >      > Do you really want the @author tag? We try to avoid them if 
possible
     >      > because they are so hard to remove, even when code/tests are 
changed
     >      > significantly.
     >
     >      No not really. It was just a part of the template I used :)
     >      @Clive: please feel free to remove the author tag.
     >
     >      > -Alan
     >
     >
     >
     >