RFR (S): 8022229: Intermittent test failures in sun/tools/jstatd

[Jaroslav Bachorik]

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-