[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [8u] RFR: 8212914: Test javax/imageio/plugins/bmp/BMP8BPPLoadTest.java fails
From:       Fairoz Matte <fairoz.matte () oracle ! com>
Date:       2018-10-30 12:47:18
Message-ID: 132381e6-df0f-47ba-9803-3322d2921ded () default
[Download RAW message or body]

Hi Jay,

Thanks for the review.

> -----Original Message-----
> From: Jayathirth D V
> Sent: Tuesday, October 30, 2018 3:01 PM
> To: Fairoz Matte <fairoz.matte@oracle.com>; Prasanta Sadhukhan
> <prasanta.sadhukhan@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.matte@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.matte@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,
> > >>
> > >> 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.matte@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,
> > >>>>
> > >>>> 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
> > >>>>>
> > >>>>>
> >
[prev in list] [next in list] [prev in thread] [next in thread]