Hi Amy,

    thanks for the comments. Updated webrev:

http://cr.openjdk.java.net/~xiaofeya/8156504/webrev.01/

-Felix
On 2016/11/2 14:18, Amy Lu wrote:
Good to see one more script test be changed to java, thank you Felix.

I'm not official reviewer, but some minor comments.

  30  * @library /lib/testlibrary
  31  * @build jdk.testlibrary.*
  32  * @compile LookupTest.java
I noticed test requires testlibrary, but seems test do not actually depend on that lib, these lines could be removed.

  77                 is.close();
Is it better to do the close in the finally block?

  128             if (serverSocket == null) {
  129                 serverSocket.close();
  130             }
Typo here?

Even more minor...
-    private static void addMappingToHostsFile (String host,
-                                               String addr,
-                                               String hostsFileName,
-                                               boolean append)
-                                             throws Exception {
+    private static void addMappingToHostsFile(String host, String addr,
+            String hostsFileName, boolean append) throws Exception {
This might be reformatted automatically by IDE, but just feel previous one is more easy to read.

Thanks,
Amy


On 11/2/16 10:39 AM, Felix Yang wrote:
Hi there,

please review the following patch for an intermittent failing test. Converted it into plain java test and avoid free port anti-pattern.

Bug:

https://bugs.openjdk.java.net/browse/JDK-8156504

Webrev:

http://cr.openjdk.java.net/~xiaofeya/8156504/webrev.00/

Thanks,

Felix



Reply via email to