RFR of JDK-8158881: Doc typo in src/../java/net/URI.java

2016-06-06 Thread Hamlin Li 

Would you please review the following simple doc patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8158881
webrev: http://cr.openjdk.java.net/~mli/8158881/webrev.00/

Thank you
-Hamlin


--- a/src/java.base/share/classes/java/net/URI.javaTue Jun 07 
10:33:38 2016 +0800
+++ b/src/java.base/share/classes/java/net/URI.javaMon Jun 06 
21:47:44 2016 -0700

@@ -183,7 +183,7 @@
  * (1)
  * 
  *
- * against the base URI {@code http://java.sun.com/j2se/1.3/} is the result
+ * against the base URI {@code http://docs.oracle.com/javase/1.3/} is 
the result

  * URI
  *
  * 
@@ -193,13 +193,13 @@
  * Resolving the relative URI
  *
  * 
- * {@code 
../../../demo/jfc/SwingSet2/src/SwingSet2.java}(2)
+ * {@code 
../../demo/jfc/SwingSet2/src/SwingSet2.java}(2)

  * 
  *
  * against this result yields, in turn,
  *
  * 
- * {@code 
http://java.sun.com/j2se/1.3/demo/jfc/SwingSet2/src/SwingSet2.java}

+ * {@code http://docs.oracle.com/demo/jfc/SwingSet2/src/SwingSet2.java}
  * 
  *

Re: RFR of JDK-8158881: Doc typo in src/../java/net/URI.java

2016-06-07 Thread Hamlin Li 


On 2016/6/7 17:34, Chris Hegarty wrote:

On 7 Jun 2016, at 05:50, Hamlin Li  wrote:

Would you please review the following simple doc patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8158881
webrev: http://cr.openjdk.java.net/~mli/8158881/webrev.00/

I’m not sure why the URL’s were ever changed from java.sun.com, since they are 
not
hyperlinks. This was a bad mistake, and badly broke the documentation, for an
unseeingly related change.

Thank you Chris for taking a look.

What about the scheme, shouldn’t that be updated to ‘https' too ?
Not sure if I understand you correctly, please correct me otherwise. 
It's not necessary to update to "https", because it's only sample code, 
and it also works well with "http".


Thank you
-Hamlin

-Chris.


Thank you
-Hamlin


--- a/src/java.base/share/classes/java/net/URI.javaTue Jun 07 10:33:38 2016 
+0800
+++ b/src/java.base/share/classes/java/net/URI.javaMon Jun 06 21:47:44 2016 
-0700
@@ -183,7 +183,7 @@
  * (1)
  * 
  *
- * against the base URI {@code http://java.sun.com/j2se/1.3/} is the result
+ * against the base URI {@code http://docs.oracle.com/javase/1.3/} is the 
result
  * URI
  *
  * 
@@ -193,13 +193,13 @@
  * Resolving the relative URI
  *
  * 
- * {@code 
../../../demo/jfc/SwingSet2/src/SwingSet2.java}(2)
+ * {@code 
../../demo/jfc/SwingSet2/src/SwingSet2.java}(2)
  * 
  *
  * against this result yields, in turn,
  *
  * 
- * {@code http://java.sun.com/j2se/1.3/demo/jfc/SwingSet2/src/SwingSet2.java}
+ * {@code http://docs.oracle.com/demo/jfc/SwingSet2/src/SwingSet2.java}
  * 
  *

Re: RFR of JDK-8158881: Doc typo in src/../java/net/URI.java

2016-06-07 Thread Hamlin Li 

On 2016/6/8 4:27, Bradford Wetmore wrote:
May I suggest using example.com [1] and an arbitrary path instead on 
these two examples, so that there's no chance of it being mistaken for 
a valid URL.  This example uses old hostnames that might change yet 
again (docs.oracle.com) and an ancient JDK platform (1.3), let's just 
remove that confusion now.

Thank you Brad.
Sure, your suggestion sounds reasonable, I should have made it more 
clear that the changed URLs in doc are not valid URLs, they just sample.

Please check webrev: http://cr.openjdk.java.net/~mli/8158881/webrev.01/

Thank you
-Hamlin


Thanks,

Brad

[1] RFC 2606:  https://tools.ietf.org/html/rfc2606

On 6/7/2016 2:34 AM, Chris Hegarty wrote:



On 7 Jun 2016, at 05:50, Hamlin Li  wrote:

Would you please review the following simple doc patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8158881
webrev: http://cr.openjdk.java.net/~mli/8158881/webrev.00/


I’m not sure why the URL’s were ever changed from java.sun.com, since 
they are not
hyperlinks. This was a bad mistake, and badly broke the 
documentation, for an

unseeingly related change.

What about the scheme, shouldn’t that be updated to ‘https' too ?

-Chris.


Thank you
-Hamlin


--- a/src/java.base/share/classes/java/net/URI.javaTue Jun 07 
10:33:38 2016 +0800
+++ b/src/java.base/share/classes/java/net/URI.javaMon Jun 06 
21:47:44 2016 -0700

@@ -183,7 +183,7 @@
 * (1)
 * 
 *
- * against the base URI {@code http://java.sun.com/j2se/1.3/} is 
the result
+ * against the base URI {@code http://docs.oracle.com/javase/1.3/} 
is the result

 * URI
 *
 * 
@@ -193,13 +193,13 @@
 * Resolving the relative URI
 *
 * 
- * {@code 
../../../demo/jfc/SwingSet2/src/SwingSet2.java}(2)
+ * {@code 
../../demo/jfc/SwingSet2/src/SwingSet2.java}(2)

 * 
 *
 * against this result yields, in turn,
 *
 * 
- * {@code 
http://java.sun.com/j2se/1.3/demo/jfc/SwingSet2/src/SwingSet2.java}
+ * {@code 
http://docs.oracle.com/demo/jfc/SwingSet2/src/SwingSet2.java}

 * 
 *

Re: RFR of JDK-8199215: Re-examine getFreePort method in test infrastructure library

2018-03-15 Thread Hamlin Li 



On 15/03/2018 7:20 PM, Alan Bateman wrote:

On 15/03/2018 08:43, Hamlin Li wrote:

:

Hi Alan,

Thank you for reviewing, I have updated the webrev in place.
( cc'ing serviceability-dev and net-dev as these are the other areas 
that use the getFreePort method in the test library. For context, the 
patch that we are discussing is:

    http://cr.openjdk.java.net/~mli/8199215/webrev.00/ )

The new implementation of getFreePort looks good but it no longer 
throws InterruptedException and so might need some of the usages (esp. 
in the serviceability tests) to be updated. Also the comment "The 
function will spin ..." is no longer relevant and can be removed.


Moving refusingEndpoint() from the NIO test to Utils looks okay. The 
"it's much more stable ..." in the method description looks a it 
inconsistent with the other wording. An alternative is "This method is 
better choice than getFreePort for tests that need an endpoint that 
refuses connections".


The update to the tests look okay to me.

Hi Alan,
Thank you for detailed reviewing. I have updated the webrev in place. 
(http://cr.openjdk.java.net/~mli/8199215/webrev.00/)


Thank you
-Hamlin


-Alan

Re: RFR of JDK-8199215: Re-examine getFreePort method in test infrastructure library

2018-03-16 Thread Hamlin Li 


On 16/03/2018 4:05 PM, Alan Bateman wrote:

On 16/03/2018 01:54, Hamlin Li wrote:

:

Hi Alan,
Thank you for detailed reviewing. I have updated the webrev in place. 
(http://cr.openjdk.java.net/~mli/8199215/webrev.00/)

Looks good, just a minor typo "is better choice" -> "is a better choice".

Hi Alan,
Thank you, I will modify it when push.


Just to confirm, have you run the serviceability and http client tests 
to make sure that they compile with this change?
Yes, I ran tier1,tier2,tier3 tests(I think it includes httpclient 
tests), and also specific tests using Utils.getFreePort in svc area.

I think I'm good to push?

Thank you
-Hamlin


-Alan

RFR of JDK-8210802,temp files left by tests in jdk/java/net/httpclient

2018-09-16 Thread Hamlin Li 

Would you please review the following patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8210802
webrev: http://cr.openjdk.java.net/~mli/8210802/webrev.00/

Thank you
-Hamlin

Re: RFR of JDK-8210802,temp files left by tests in jdk/java/net/httpclient

2018-09-17 Thread Hamlin Li 

Hi Christoph,

Thank you for review.

Normally I prefer to use "finally" too.

But for this case, it's simpler to use deleteOnExit(), and for 
TestUtil.java and Driver.java it can not use "finally", because the temp 
files will be used outside of methods.


If you insist on "finally" I can modify them except of TestUtil.java and 
Driver.java.


Thank you

-Hamlin


On 2018/9/17 4:25 PM, Langer, Christoph wrote:

Hi Hamlin,

wouldn't it be better/cleaner to move the deletion of files into finally 
blocks? But I guess one can do it with deleteOnExit() as well...

Best regards
Christoph


-Original Message-----
From: net-dev  On Behalf Of Hamlin Li
Sent: Montag, 17. September 2018 07:55
To: OpenJDK Network Dev list 
Subject: RFR of JDK-8210802,temp files left by tests in jdk/java/net/httpclient

Would you please review the following patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8210802
webrev: http://cr.openjdk.java.net/~mli/8210802/webrev.00/

Thank you
-Hamlin

Re: RFR of JDK-8210802,temp files left by tests in jdk/java/net/httpclient

2018-09-17 Thread Hamlin Li 

Hi Chris,

Thank you for review. I agree with you.

I think we only need to address the issue in EchoHandler.java, where the 
temp file is created in default path rather than the path in test scratch.


updated webrev: http://cr.openjdk.java.net/~mli/8210802/webrev.01/

Thank you

-Hamlin


On 2018/9/17 6:05 PM, Chris Hegarty wrote:


Cleaning up in tests is always problematic, since tests
often run in environments where such cleaning actions
are not performed immediately. Just look at the hoops
that we go through to delete files ( and directories )
in  test/lib/jdk/test/lib/util/FileUtils.java, that I
added a few years ago. That said, we should try to clean
up whenever possible.

In this particular case the files in question are
created in the jtreg's scratch directory. If a test
fails it could be useful, for diagnostic purposes, to
inspect the contents of such files, no? Are these
files causing issues? How invasive a change would it be
to only delete the files if a test fails?

-Chris.


On 17/09/18 10:11, Langer, Christoph wrote:

...


On 2018/9/17 4:25 PM, Langer, Christoph wrote:

Hi Hamlin,

wouldn't it be better/cleaner to move the deletion of files into 
finally

blocks? But I guess one can do it with deleteOnExit() as well...


Best regards
Christoph


-Original Message-
From: net-dev  On Behalf Of

Hamlin Li

Sent: Montag, 17. September 2018 07:55
To: OpenJDK Network Dev list 
Subject: RFR of JDK-8210802,temp files left by tests in

jdk/java/net/httpclient


Would you please review the following patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8210802
webrev: http://cr.openjdk.java.net/~mli/8210802/webrev.00/

Thank you
-Hamlin