RFR(L): 8071908, 8071909: Port internal Diagnostic Command tests and test framework to jtreg (plus some testlibrary changes)

[Jaroslav Bachorik]

Nice! For me it's good to go.

-JB-

On 30.1.2015 20:32, Mikael Auno wrote:
> Jaroslav,
>
> First of all, thanks for the quick response and sorry for my slow one
> (your message didn't sort into the mail folder I expected, so didn't see
> it until now).
>
> Secondly, all good comments; all fixed.
>
> Updated webrev:
> http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.02/
>
> Thanks,
> Mikael
>
> On 2015-01-30 11:18, Jaroslav Bachorik wrote:
>> Hi Mikael,
>>
>> it's great to see this moving forward!
>>
>> Comments follow:
>>
>> * instead of throwing a RuntimeException from within the test classes
>> you could use o.testng.Assert.fail(...) method
>>
>> * all the newly introduced tests are missing @summary
>>
>> * test/serviceability/dcmd/compiler/CodelistTest.java
>>    L43 - unused import
>>    L68 - an unused, commented out, line
>>
>> * test/testlibrary/com/oracle/java/testlibrary/OutputAnalyzer.java
>>    L436-438 - use Arrays.asList()
>>
>> * test/serviceability/dcmd/vm/UptimeTest.java
>>    L44 - spurious wakeups may cause the test fail intermittently; should
>> make sure the wait was at least 'someUptime' seconds long
>>
>>
>> -JB-
>>
>> On 30.1.2015 10:44, Mikael Auno wrote:
>>> Hi, could I please some reviews for this test port?
>>>
>>> Issues: https://bugs.openjdk.java.net/browse/JDK-8071908
>>>           https://bugs.openjdk.java.net/browse/JDK-8071909
>>> Webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.00/
>>>
>>> Read on for the rationale on a few questions that might arise.
>>>
>>> * Why two issues?
>>>
>>> These changes are mainly a port of the Diagnostic Command (DCMD) tests
>>> and corresponding framework/utility classes from an internal (non-open)
>>> test framework to jtreg. The reason for the two issues is that the
>>> changes depend on a few modifications to testlibrary that are available
>>> in jdk/test but not yet in hotspot/test, as well as a small new addition
>>> to OutputAnalyzer, that are not specific to main subject (i.e. the DCMD
>>> tests). To keep the history as clean and coherent as possible, those
>>> changes will go in under JDK-8071909 while the new tests and
>>> DCMD-related additions to testlibrary go in under JDK-8071908.
>>>
>>> * Isn't there already utility classes for calling Diagnostic Commands?
>>>
>>> The main idea with the new utility classes in testlibrary is to provide
>>> a single interface to call Diagnostic Commands from tests, regardless of
>>> the transport used (e.g. jcmd or JMX). There are a few tests scattered
>>> around jdk/test and hotspot/test today that already utilize Diagnostic
>>> Commands in some way, but they either use different utility classes for
>>> this in different places or just do it directly in the test. Also, some
>>> of these utility classes or tests go through jcmd and some through JMX
>>> (most often without any real requirement for one transport over the
>>> other in the test). All this means that there are, today, numerous
>>> different implementations for calling Diagnostic Commands, and
>>> consequently a lot of code duplication. These utility classes are meant
>>> to replace all of these implementations, and with a single interface
>>> regardless of the transport at that.
>>>
>>> * You've missed this or that test using one of the existing utility
>>> classes!
>>>
>>> This is "by design". In order to keep the change at a more manageable
>>> size and to get the core of this change in sooner, we've chosen to do
>>> this transition in multiple steps. The first of those steps is what is
>>> in this review request; the core utility classes, the tests ported from
>>> the internal test framework and the adaption of the tests already in
>>> hotspot/test/serviceability/dcmd (since they happened to reside in the
>>> directory where we wanted to put the ported tests). When this is
>>> integrated and have gone a few rounds through nightly testing, the
>>> adaption of other tests in hotspot/test will follow, and after that
>>> jdk/test.
>>>
>>> Thanks,
>>> Mikael
>>>
>>
>