Hi Jay, Thanks for the review.
> -----Original Message----- > From: Jayathirth D V > Sent: Tuesday, October 30, 2018 3:01 PM > To: Fairoz Matte <fairoz.ma...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com>; 2d-dev@openjdk.java.net > Subject: RE: [OpenJDK 2D-Dev] [8u] RFR: 8212914: Test > javax/imageio/plugins/bmp/BMP8BPPLoadTest.java fails > > Hi Fairoz, > > @requires tag usage in jtreg : https://openjdk.java.net/jtreg/tag- > spec.html#requires_names > I think jtreg tag doesn’t support mentioning input file. Also I don’t know why > it was mentioned as "@requires BMP8BPPLoadTest.PNG" when input file in > code was "new File("BMP8BPPLoadTest.bmp")". "@requires BMP8BPPLoadTest.PNG", has been removed since webrev.00. > > Anyway since we are using stream there will be no file input issues. > Apart from what Prasanta has mentioned over webrev.02, please remove > println statements from test case before pushing. Sure, I will do that, thanks for the review. Below is the final webrev with updated changes (suggested by you and Prasanta) http://cr.openjdk.java.net/~fmatte/8212914/webrev.03/ Thanks, Fairoz > > Changes are fine. > > Thanks, > Jay > > -----Original Message----- > From: Fairoz Matte > Sent: Tuesday, October 30, 2018 1:33 PM > To: Prasanta Sadhukhan; 2d-dev@openjdk.java.net > Subject: Re: [OpenJDK 2D-Dev] [8u] RFR: 8212914: Test > javax/imageio/plugins/bmp/BMP8BPPLoadTest.java fails > > Thanks Prasanta, > > I will update that, need one more review on this? > > Thanks, > Fairoz > > > -----Original Message----- > > From: Prasanta Sadhukhan > > Sent: Tuesday, October 30, 2018 12:24 PM > > To: Fairoz Matte <fairoz.ma...@oracle.com>; 2d-dev@openjdk.java.net > > Subject: Re: [OpenJDK 2D-Dev] [8u] RFR: 8212914: Test > > javax/imageio/plugins/bmp/BMP8BPPLoadTest.java fails > > > > Hi Fairoz, > > > > @bug tag should not spearate bugs by commas, just by spaces. other > > than that, looks ok to me. > > > > Regards > > Prasanta > > On 30-Oct-18 10:13 AM, Fairoz Matte wrote: > > > Hi Prasanta, > > > > > >> -----Original Message----- > > >> From: Prasanta Sadhukhan > > >> Sent: Monday, October 29, 2018 12:18 PM > > >> To: Fairoz Matte <fairoz.ma...@oracle.com>; 2d- > d...@openjdk.java.net > > >> Subject: Re: [OpenJDK 2D-Dev] [8u] RFR: 8212914: Test > > >> javax/imageio/plugins/bmp/BMP8BPPLoadTest.java fails > > >> > > >> Hi Fairoz, > > >> > > >> I do not see ImageIO.read and ImageIO.createImageInputStream > > throwing > > >> IOB exception in the spec, it throws IOException so I guess there's > > >> no point catching IOB. > > > This test case has been added as part of "JDK-8182461: > > IndexOutOfBoundsException when reading indexed color BMP" > > > Due to missing "break" IOB exception was generated. > > > > > >> Also, you need to add this bugid to @bug tag and also remove > > >> @author tag which we do not recommend now. Also, can you please > > >> indent byte[] > > data ? > > > Yes updated all, here is the updated webrev. > > > http://cr.openjdk.java.net/~fmatte/8212914/webrev.02/ > > > > > > Thanks, > > > Fairoz > > >> Regards > > >> Prasanta > > >> On 26-Oct-18 10:03 PM, Fairoz Matte wrote: > > >>> Hi Prasanta, > > >>> > > >>> Thanks for looking into it. > > >>> > > >>>> -----Original Message----- > > >>>> From: Prasanta Sadhukhan > > >>>> Sent: Friday, October 26, 2018 8:56 AM > > >>>> To: Fairoz Matte <fairoz.ma...@oracle.com>; 2d- > > d...@openjdk.java.net > > >>>> Subject: Re: [OpenJDK 2D-Dev] [8u] RFR: 8212914: Test > > >>>> javax/imageio/plugins/bmp/BMP8BPPLoadTest.java fails > > >>>> > > >>>> Hi Fairoz, > > >>>> > > >>>> Do you know if the bmp image file has Oracle copyright? If not, > > >>>> you cannot check it in. > > >>> I was not aware of it. Yes image file is not Oracle copyright compliant. > > >>> > > >>>> Alternatively, you may get a hexdump of the bmp file and create a > > >>>> byte[] array with that hex data and create ByteArrayInputStream > > >>>> with that and use that for ImageIO as an ImageInputStream. > > >>> Yes it is good approach, here is the updated webrev > > >>> http://cr.openjdk.java.net/~fmatte/8212914/webrev.01/ > > >>> > > >>> Thanks, > > >>> Fairoz > > >>>> Regards > > >>>> Prasanta > > >>>> On 25-Oct-18 11:52 AM, Fairoz Matte wrote: > > >>>>> Hi, > > >>>>> > > >>>>> Kindly review the small fix. > > >>>>> > > >>>>> Background: > > >>>>> "javax/imageio/plugins/bmp/BMP8BPPLoadTest.java test case" has > > >> been > > >>>>> added part of JDK-8182461, Test case has a dependency on input > > >>>>> file > > >>>> "BMP8BPPLoadTest.PNG", during push this was missed. > > >>>>> In this fix test case also modified to refer the input file from > > >>>>> same directory > > >>>>> > > >>>>> JBS bug - https://bugs.openjdk.java.net/browse/JDK-8212914 > > >>>>> Webrev - http://cr.openjdk.java.net/~fmatte/8212914/webrev.00/ > > >>>>> > > >>>>> Thanks, > > >>>>> Fairoz > > >>>>> > > >>>>> > >
