RFR 8166139/10, Refactor java/net shell cases to java
Felix Yang
felix.yang at oracle.com
Fri May 26 04:46:40 UTC 2017
Hi Amy,
it is intended, because in my changes CompilerUtils is usually used
together with JarUtils, which has not been moved yet. It is a bit
confusing to add two lib directories, and even there are files with the
same name. So it may be clearer to refactor lib usage unified in
JDK-8075327 <https://bugs.openjdk.java.net/browse/JDK-8075327> or
clean-ups tasks with single purpose.
Thanks,
Felix
On 2017/5/26 11:43, Amy Lu wrote:
> Hi, Felix
>
> I noticed that CompilerUtils from jdk/test/lib/testlibrary is still
> used in tests. It’s better to use the version [1] from top level.
>
> (Not a reviewer.)
>
> Thanks,
> Amy
>
> [1]
> http://hg.openjdk.java.net/jdk10/jdk10/file/tip/test/lib/jdk/test/lib/compiler/CompilerUtils.java
>
> On 5/26/17 11:14 AM, 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.
>>
>> -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
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20170526/304a4b3c/attachment.html>
More information about the net-dev
mailing list