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

Hi Chris, pushed, thank you very much! -Felix On 2017/5/30 17:44, Chris Hegarty wrote: On 29/05/17 16:17, Felix Yang wrote: Hi Chris, please review the updated webrev below. Comments inline. http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.03/ Thanks Felix. .. 2) test/java/net

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

On 29/05/17 16:17, Felix Yang wrote: Hi Chris, please review the updated webrev below. Comments inline. http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.03/ Thanks Felix. .. 2) test/java/net/URLClassLoader/getresourceasstream/policy Please add a copyright/header. Checked exist

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

Hi Chris, please review the updated webrev below. Comments inline. http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.03/ -Felix On 2017/5/29 18:19, Chris Hegarty wrote: Felix, Thanks for taking this one. A few comments: 1) test/java/net/URLConnection/6212146/TestDriver.java Please

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

Felix, Thanks for taking this one. A few comments: 1) test/java/net/URLConnection/6212146/TestDriver.java Please check indentation around L81: 80 Files.copy(testJar, ___targetDir.resolve(ARCHIVE_NAME), 81 ___StandardCopyOption.REPLACE_EXISTIN

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

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.

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

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/GetResourc

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

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

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

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

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

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

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

Hi Felix, closetest/CloseTest: 32: Please put the @modules directives together (and not repeat) 44: do not use wildcard imports (reconfigure your IDE if necessary to avoid accidents). 63: setup() is invoked twice... remove 1 or explain why 2 calls 69, 78,90,91,... : space in method call

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

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