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




Reply via email to