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

Felix Yang felix.yang at oracle.com
Sat May 27 01:15:16 UTC 2017


Hi Roger,

     thanks for the comments. Please see the new webrev

http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.02/

-Felix
On 2017/5/27 3:50, Roger Riggs wrote:
> 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
Removed such check, since the test depends on several system 
properties("test.src", "user.dir"). It is not expected to run outside jtreg.

-Felix
>
> 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