On 15.10.2013 15:35, Yekaterina Kantserova wrote:
Hi,
Could I please have a review of this fix.
The purpose of this fix is to get rid of intermittent failures in
sun/tools/jstatd tests and make the tests more stable, legible and
maintainable.
Thanks,
Katja
Webrev:
http://cr.openjdk.java.net/~ykantser/8022229/webrev.00/
Primal bug:
https://bugs.openjdk.java.net/browse/JDK-8022229
Similar bugs:
https://bugs.openjdk.java.net/browse/JDK-8019630
https://bugs.openjdk.java.net/browse/JDK-6636094
https://bugs.openjdk.java.net/browse/JDK-6543979
Hi Katja,
these are my comments:
test/sun/tools/jstatd/JstatdHelper.java
=======================================
* 82-84 Should be moved to 91 since the check is only valid for
Tool.RMIREGISTRY
* Started processes are not cleaned up in case they get stuck for some
reason. The original test tried to explicitely kill the target processes.
test/lib/testlibrary/jdk/testlibrary/Asserts.java
=================================================
* Missing javadoc for the public library methods
test/lib/testlibrary/jdk/testlibrary/ProcessThread.java
=======================================================
* Missing javadoc for the public library methods
test/lib/testlibrary/jdk/testlibrary/TestThread.java
====================================================
* Missing javadoc for the public library methods
test/lib/testlibrary/jdk/testlibrary/Utils.java
===============================================
* Missing javadoc for the public library methods
* 57 serverSocket.close() should be called in "finally" block to make
sure the resources are released even in case of an exception being thrown
test/sun/tools/jstatd/JstatGcutilParser.java
============================================
* The comments from the original test explaining the tested format are
missing
Cheers,
-JB-