RFR 8166139/10, Refactor java/net shell cases to java

Roger Riggs Roger.Riggs at Oracle.com
Fri May 26 19:50:46 UTC 2017


Hi Felix,

fyi, there is a new version of webrev.ksh that provides convenient next 
and previous links.
http://hg.openjdk.java.net/code-tools/webrev/raw-file/d26c194772db/webrev.ksh


CloseTest.java: 66; checking WORK_DIR *after* calling setup() does not 
make sense.

Ditto: closetest/GetResourceAsStream.java

URLConnection/6212146/Test.java:47 typo: ULR -> URL

Regards, Roger


On 5/25/2017 11:14 PM, Felix Yang wrote:
> Hi Roger,
>
>     please review the updated webrev:
>
> http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/
>
> Thanks,
> Felix
> On 2017/5/26 3:24, Roger Riggs wrote:
>> Hi Felix,
>>
>> closetest/CloseTest:
>>
>> 32: Please put the @modules directives together (and not repeat)
> Fixed
> -Felix
>>
>> 44: do not use wildcard imports  (reconfigure your IDE if necessary 
>> to avoid accidents).
> Fixed and also organized imports in other test files
> -Felix
>>
>> 63: setup() is invoked twice...  remove 1 or explain why 2 calls
> Fixed
> -Felix
>>
>> 69, 78,90,91,... :  space in method call is not proper style
> Fixed and also formatted other history codes. Just format only without 
> logic change.
> Not restricted to this test. I only formatted history codes "nearby", 
> to avoid hiding real logic changes in the patch.
>
> -Felix
>>
>> sealing/CheckSealed.java:
>>  - Why is @test block removed?
>>     Should be converted to @run main CheckSealed
> There was a checksealed.sh. I replaced it with CheckSealedTest.java 
> and declared @test there.
> http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/test/java/net/URLClassLoader/sealing/CheckSealedTest.java.html 
>
>
> BTW, you may noticed test/java/net/URLClassLoader/B5077773.* were 
> removed. That is because corresponding prerequisite was removed even 
> in JDK 8.
Perhaps then add a comment to CheckSealed.java indicating it is compiled 
and executed by CheckSealedTest.java.

Roger

>
> -Felix
>>
>> Regards, Roger
>>
>>
>>
>> On 5/25/2017 4:08 AM, Felix Yang wrote:
>>> Hi there,
>>>
>>>    please review following patch to convert all shell cases under 
>>> java/net to plain java codes.
>>>
>>> Webrev:
>>>
>>>     http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.00/
>>>
>>> Bug:
>>>
>>>     https://bugs.openjdk.java.net/browse/JDK-8166139
>>>
>>>
>>> Thanks,
>>>
>>> Felix
>>>
>>
>



More information about the core-libs-dev mailing list