Hi Roger,
Thank you for reviewing.
Please check the new webrev:
http://cr.openjdk.java.net/~mli/8085192/webrev.01/, and comments inline
below.
On 2016/9/27 23:14, Roger Riggs wrote:
Hi Hamlin,
Marking each test that uses RMID.launch with the bugid does not seem
to be meaningful
since the bug is in the support infrastructure of the test and not
specific to the test itself.
It would be overkill to try to confirm the bug was fixed by running
all those tests.
Putting the bugid on 1 of the tests would be sufficient.
Remove all bug ids except of the one in CheckImplClassLoader.java.
JavaVM.java:
- 134-138: Why define these private methods if they are not going to
be used in outputContains?
- 185: Its inefficient to convert the byte array to a string for when
looking for each string.
It would be cleaner for JavaVM to have public outputString and
errorString methods
and check them separately in RMID.
(remove the JavaVM.outputContains method which hides which stream
the string appeared in).
Fixed.
(It would be a bigger change to change this to use the test library
ProcessTools and OutputAnalyzer).
Thank you suggestion. Yes, you're right, it will be a little bit
complicated to use ProcessTools and OutputAnalyzer in this situation,
need to modify these classes, because they will block until process
terminates. So I prefer to use current implementation, as it's simple
and clear.
Thank you
-Hamlin
Roger
On 9/27/2016 5:22 AM, Hamlin Li wrote:
Please review the fix for JDK-8085192. The fix checks whether it
fails to launch rmid due to "Port already in use" error, it will
launch rmid again and again(20 times at most) until no such issue.
bug:
https://bugs.openjdk.java.net/browse/JDK-8085192
webrev:
http://cr.openjdk.java.net/~mli/8085192/webrev.00/
Thank you
-Hamlin
