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-

Reply via email to