Re: RFR:JDK-8075205 java/net/URLClassLoader/closetest/CloseTest.java and GetResourceAsStream.java failed with existing dir

[Roger Riggs]

Hi,

Looks good.

Editorial improvements:

GetResourceAsStream.java:

 - line 44-52:  Add the new variables after the test description like 
CloseTest.java or change CloseTest

   to improve style consistency.

 - Line 54-57: fix indentation

 - consistently use /* ... */ or // comment conventions for methods.

Thanks, Roger





On 10/25/2016 10:50 PM, Srinivasan Raghavan wrote:

Hi all

Please help with one more reviewer comment.

Regards
Srinivasan Raghavan

On 19-Oct-2016, at 9:38 AM, Srinivasan Raghavan 
 wrote:

Thanks for the review.

On 18-Oct-2016, at 4:16 PM, Chris Hegarty  wrote:



On 17 Oct 2016, at 09:51, Srinivasan Raghavan  
wrote:

Hi all

Please review the fix for the bug

Bug :https://bugs.openjdk.java.net/browse/JDK-8075205

The tests uses classes directory for the output files. This can result in the 
files being left over after the test is complete which can result in 
instability. The tests copies the files to be compiled form test src to test 
classes which can result in copy of permission and result in instability 
because the test has delete operations. The test fails randomly mostly in copy 
or delete operation. I propose the test to be refactored to make the use 
scratch directory as its output directory and eliminate shell by using 
testlibrary utils.

fix : http://cr.openjdk.java.net/~sraghavan/8075205/webrev.00/

This looks good to me. Thanks Srinivasan.

-Chris.

Re: RFR:JDK-8075205 java/net/URLClassLoader/closetest/CloseTest.java and GetResourceAsStream.java failed with existing dir

[Srinivasan Raghavan]

Hi all

Please help with one more reviewer comment.

Regards
Srinivasan Raghavan
> On 19-Oct-2016, at 9:38 AM, Srinivasan Raghavan 
>  wrote:
> 
> Thanks for the review.
>> On 18-Oct-2016, at 4:16 PM, Chris Hegarty  wrote:
>> 
>> 
>>> On 17 Oct 2016, at 09:51, Srinivasan Raghavan 
>>>  wrote:
>>> 
>>> Hi all
>>> 
>>> Please review the fix for the bug 
>>> 
>>> Bug :https://bugs.openjdk.java.net/browse/JDK-8075205
>>> 
>>> The tests uses classes directory for the output files. This can result in 
>>> the files being left over after the test is complete which can result in 
>>> instability. The tests copies the files to be compiled form test src to 
>>> test classes which can result in copy of permission and result in 
>>> instability because the test has delete operations. The test fails randomly 
>>> mostly in copy or delete operation. I propose the test to be refactored to 
>>> make the use scratch directory as its output directory and eliminate shell 
>>> by using testlibrary utils.
>>> 
>>> fix : http://cr.openjdk.java.net/~sraghavan/8075205/webrev.00/
>> 
>> This looks good to me. Thanks Srinivasan.
>> 
>> -Chris.
> 

Re: RFR:JDK-8075205 java/net/URLClassLoader/closetest/CloseTest.java and GetResourceAsStream.java failed with existing dir

[Srinivasan Raghavan]

Thanks for the review.
> On 18-Oct-2016, at 4:16 PM, Chris Hegarty  wrote:
> 
> 
>> On 17 Oct 2016, at 09:51, Srinivasan Raghavan 
>>  wrote:
>> 
>> Hi all
>> 
>> Please review the fix for the bug 
>> 
>> Bug :https://bugs.openjdk.java.net/browse/JDK-8075205
>> 
>> The tests uses classes directory for the output files. This can result in 
>> the files being left over after the test is complete which can result in 
>> instability. The tests copies the files to be compiled form test src to test 
>> classes which can result in copy of permission and result in instability 
>> because the test has delete operations. The test fails randomly mostly in 
>> copy or delete operation. I propose the test to be refactored to make the 
>> use scratch directory as its output directory and eliminate shell by using 
>> testlibrary utils.
>> 
>> fix : http://cr.openjdk.java.net/~sraghavan/8075205/webrev.00/
> 
> This looks good to me. Thanks Srinivasan.
> 
> -Chris.

Re: RFR:JDK-8075205 java/net/URLClassLoader/closetest/CloseTest.java and GetResourceAsStream.java failed with existing dir

[Chris Hegarty]

> On 17 Oct 2016, at 09:51, Srinivasan Raghavan 
>  wrote:
> 
> Hi all
> 
> Please review the fix for the bug 
> 
> Bug :https://bugs.openjdk.java.net/browse/JDK-8075205
> 
> The tests uses classes directory for the output files. This can result in the 
> files being left over after the test is complete which can result in 
> instability. The tests copies the files to be compiled form test src to test 
> classes which can result in copy of permission and result in instability 
> because the test has delete operations. The test fails randomly mostly in 
> copy or delete operation. I propose the test to be refactored to make the use 
> scratch directory as its output directory and eliminate shell by using 
> testlibrary utils.
> 
> fix : http://cr.openjdk.java.net/~sraghavan/8075205/webrev.00/

This looks good to me. Thanks Srinivasan.

-Chris.

RFR:JDK-8075205 java/net/URLClassLoader/closetest/CloseTest.java and GetResourceAsStream.java failed with existing dir

[Srinivasan Raghavan]

Hi all

Please review the fix for the bug 

Bug :https://bugs.openjdk.java.net/browse/JDK-8075205

The tests uses classes directory for the output files. This can result in the 
files being left over after the test is complete which can result in 
instability. The tests copies the files to be compiled form test src to test 
classes which can result in copy of permission and result in instability 
because the test has delete operations. The test fails randomly mostly in copy 
or delete operation. I propose the test to be refactored to make the use 
scratch directory as its output directory and eliminate shell by using 
testlibrary utils.

fix : http://cr.openjdk.java.net/~sraghavan/8075205/webrev.00/

Regards
Srinivasan Raghavan