[9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

Sergey Bylokhov
Hello,
Please review the fix for jdk9.

This fix is a part of the effort to opensource the tests in jdk.
In the fix a number of tests for ImageIO are moved to the open ws.
Note that most of the code moved as-is, and probably can be improved in the future, but I tried to limit the number of changes in this move.

Bug: https://bugs.openjdk.java.net/browse/JDK-8177628
Webrev can be found at: http://cr.openjdk.java.net/~serb/8177628/webrev.00
The same fix for JavaSound: http://mail.openjdk.java.net/pipermail/sound-dev/2016-October/000472.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

Prahalad kumar Narayanan
Hello Sergey

The patch looks good to me.
    . I imported the patch and executed automated jtreg tests.
    . No failures were noticed on win64 build.

One subtle observation
    . One of the test cases- DestTypeTest.java, seems to require VancouverIsland200.jpg.
    . Since the file is looked up in a try ... catch block, the test will continue to execute despite image not being a part of the patch.
    . You could check this test code & copyrights for the image just to ensure the intended functionality is tested.

Thanks
Have a good day

Prahalad N.

-----Original Message-----
From: Sergey Bylokhov
Sent: Saturday, May 20, 2017 7:51 AM
To: Jayathirth D V; Philip R Race
Cc: 2d-Dev
Subject: [OpenJDK 2D-Dev] [9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

Hello,
Please review the fix for jdk9.

This fix is a part of the effort to opensource the tests in jdk.
In the fix a number of tests for ImageIO are moved to the open ws.
Note that most of the code moved as-is, and probably can be improved in the future, but I tried to limit the number of changes in this move.

Bug: https://bugs.openjdk.java.net/browse/JDK-8177628
Webrev can be found at: http://cr.openjdk.java.net/~serb/8177628/webrev.00
The same fix for JavaSound: http://mail.openjdk.java.net/pipermail/sound-dev/2016-October/000472.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

Sergey Bylokhov
In reply to this post by Sergey Bylokhov
Hi, Prahalad.
Thank you for review.
Yes, this image(if found) is used to initialize the BImage, but the test can generates BImage on its own.
In the new version the usage of ".jpg" is removed:
http://cr.openjdk.java.net/~serb/8177628/webrev.01

I have checked that the test fails on jdk5 where related bug was not fixed.

----- [hidden email] wrote:

> Hello Sergey
>
> The patch looks good to me.
>     . I imported the patch and executed automated jtreg tests.
>     . No failures were noticed on win64 build.
>
> One subtle observation
>     . One of the test cases- DestTypeTest.java, seems to require
> VancouverIsland200.jpg.
>     . Since the file is looked up in a try ... catch block, the test
> will continue to execute despite image not being a part of the patch.
>     . You could check this test code & copyrights for the image just
> to ensure the intended functionality is tested.
>
> Thanks
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Saturday, May 20, 2017 7:51 AM
> To: Jayathirth D V; Philip R Race
> Cc: 2d-Dev
> Subject: [OpenJDK 2D-Dev] [9] Review Request: 8177628 Opensource
> unit/regression tests for ImageIO
>
> Hello,
> Please review the fix for jdk9.
>
> This fix is a part of the effort to opensource the tests in jdk.
> In the fix a number of tests for ImageIO are moved to the open ws.
> Note that most of the code moved as-is, and probably can be improved
> in the future, but I tried to limit the number of changes in this
> move.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8177628
> Webrev can be found at:
> http://cr.openjdk.java.net/~serb/8177628/webrev.00
> The same fix for JavaSound:
> http://mail.openjdk.java.net/pipermail/sound-dev/2016-October/000472.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

Philip Race
+1

-phil.

On 05/22/2017 02:46 PM, Sergey Bylokhov wrote:

> Hi, Prahalad.
> Thank you for review.
> Yes, this image(if found) is used to initialize the BImage, but the test can generates BImage on its own.
> In the new version the usage of ".jpg" is removed:
> http://cr.openjdk.java.net/~serb/8177628/webrev.01
>
> I have checked that the test fails on jdk5 where related bug was not fixed.
>
> ----- [hidden email] wrote:
>
>> Hello Sergey
>>
>> The patch looks good to me.
>>      . I imported the patch and executed automated jtreg tests.
>>      . No failures were noticed on win64 build.
>>
>> One subtle observation
>>      . One of the test cases- DestTypeTest.java, seems to require
>> VancouverIsland200.jpg.
>>      . Since the file is looked up in a try ... catch block, the test
>> will continue to execute despite image not being a part of the patch.
>>      . You could check this test code & copyrights for the image just
>> to ensure the intended functionality is tested.
>>
>> Thanks
>> Have a good day
>>
>> Prahalad N.
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Saturday, May 20, 2017 7:51 AM
>> To: Jayathirth D V; Philip R Race
>> Cc: 2d-Dev
>> Subject: [OpenJDK 2D-Dev] [9] Review Request: 8177628 Opensource
>> unit/regression tests for ImageIO
>>
>> Hello,
>> Please review the fix for jdk9.
>>
>> This fix is a part of the effort to opensource the tests in jdk.
>> In the fix a number of tests for ImageIO are moved to the open ws.
>> Note that most of the code moved as-is, and probably can be improved
>> in the future, but I tried to limit the number of changes in this
>> move.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8177628
>> Webrev can be found at:
>> http://cr.openjdk.java.net/~serb/8177628/webrev.00
>> The same fix for JavaSound:
>> http://mail.openjdk.java.net/pipermail/sound-dev/2016-October/000472.html

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

Prahalad kumar Narayanan
In reply to this post by Sergey Bylokhov
Hello Sergey

The changes look good.

Thanks
Have a good day

Prahalad N.

-----Original Message-----
From: Sergey Bylokhov
Sent: Tuesday, May 23, 2017 3:16 AM
To: Prahalad Kumar Narayanan
Cc: Jayathirth D V; [hidden email]; Philip Race
Subject: Re: [OpenJDK 2D-Dev] [9] Review Request: 8177628 Opensource unit/regression tests for ImageIO

Hi, Prahalad.
Thank you for review.
Yes, this image(if found) is used to initialize the BImage, but the test can generates BImage on its own.
In the new version the usage of ".jpg" is removed:
http://cr.openjdk.java.net/~serb/8177628/webrev.01

I have checked that the test fails on jdk5 where related bug was not fixed.

----- [hidden email] wrote:

> Hello Sergey
>
> The patch looks good to me.
>     . I imported the patch and executed automated jtreg tests.
>     . No failures were noticed on win64 build.
>
> One subtle observation
>     . One of the test cases- DestTypeTest.java, seems to require
> VancouverIsland200.jpg.
>     . Since the file is looked up in a try ... catch block, the test
> will continue to execute despite image not being a part of the patch.
>     . You could check this test code & copyrights for the image just
> to ensure the intended functionality is tested.
>
> Thanks
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Saturday, May 20, 2017 7:51 AM
> To: Jayathirth D V; Philip R Race
> Cc: 2d-Dev
> Subject: [OpenJDK 2D-Dev] [9] Review Request: 8177628 Opensource
> unit/regression tests for ImageIO
>
> Hello,
> Please review the fix for jdk9.
>
> This fix is a part of the effort to opensource the tests in jdk.
> In the fix a number of tests for ImageIO are moved to the open ws.
> Note that most of the code moved as-is, and probably can be improved
> in the future, but I tried to limit the number of changes in this
> move.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8177628
> Webrev can be found at:
> http://cr.openjdk.java.net/~serb/8177628/webrev.00
> The same fix for JavaSound:
> http://mail.openjdk.java.net/pipermail/sound-dev/2016-October/000472.h
> tml
Loading...