RFR 8014506: Test for Jdp (Java Discovery Protocol) feature

[Mikael Auno]

The reduction in testsuites in JdpTest.sh is due to those "testsuites" 
removed being ported to Java with this change. The last "testsuite" in 
JdpTest.sh however, has not been ported yet, therefore JdpTest.sh can't 
be removed completely.

Mikael

On 2013-11-11 14:19, Staffan Larsen wrote:
> Why are there changes to the existing tests? Most look like whitespace
> changes, but JdpTest.sh changes the “testsuites” that are being run.
>
> /Staffan
>
> On 08 Nov 2013, at 18:45, Mikael Auno <mikael.auno at oracle.com
> <mailto:mikael.auno at oracle.com>> wrote:
>
>> There was some unintended whitespace changes in webrev.05 that Alex
>> had missed. I removed them and uploaded webrev.06 for him.
>>
>> http://cr.openjdk.java.net/~miauno/8014506/webrev.06/
>>
>> Mikael
>>
>> On 2013-11-08 17:02, Alex Schenkman wrote:
>>> Hi list,
>>>
>>> Latest version is up for review here [1].
>>> It fixes Staffan's latest comments (see below), as well as some
>>> indentation and tab/spaces nits.
>>>
>>> Thanks!
>>>
>>> [1]http://cr.openjdk.java.net/~miauno/8014506/
>>> <http://cr.openjdk.java.net/%7Emiauno/8014506/>
>>>
>>> On 2013-10-30 16:03, Staffan Larsen wrote:
>>>> Alex,
>>>>
>>>> This looks much better! Thanks for spending the time.
>>>>
>>>> README:
>>>>  nit: contribuited -> contributed
>>>>
>>>> JdpTestUtilTest.java:
>>>>  Can you add a comment saying this is a test for the tests - just
>>>> like PacketTest.java and PortAlreadyInUseTest.java?
>>>>
>>>> PortFinder.java:
>>>>  Please remove this and use Utils.getFreePort() since ykantser's fix
>>>> is now pushed.
>>>>
>>>> Thanks,
>>>> /Staffan
>>>>
>>>> On 29 okt 2013, at 15:53, Alex Schenkman <alex.schenkman at oracle.com
>>>> <mailto:alex.schenkman at oracle.com>
>>>> <mailto:alex.schenkman at oracle.com>> wrote:
>>>>
>>>>> Hi Staffan,
>>>>>
>>>>> There is a new webrev here [1], addressing your comments.
>>>>>
>>>>> The Jdp specs have changed a bit, adding three optional fields to the
>>>>> Jdp packet [2].
>>>>> The JEP-168 [3] is not updated yet, but Dmitry will do it soon.
>>>>> This enhancement request is tracking the changes needed for SQE [4].
>>>>>
>>>>> See inlined answers for details on your previos comments, please.
>>>>>
>>>>> Thanks!
>>>>>
>>>>>
>>>>> [1]
>>>>> http://cr.openjdk.java.net/~dsamersoff/sponsorship/aschenkman/8014506/webrev.04/
>>>>> <http://cr.openjdk.java.net/%7Edsamersoff/sponsorship/aschenkman/8014506/webrev.04/>
>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8004213
>>>>> [3] http://openjdk.java.net/jeps/168
>>>>> [4] https://bugs.openjdk.java.net/browse/JDK-8027436
>>>>>
>>>>> On 2013-10-24 10:54, Staffan Larsen wrote:
>>>>>> Alex,
>>>>>>
>>>>>> test/sun/management/jdp/README
>>>>>>  Thanks for providing a README!
>>>>>>  References to JDK-7169888 seem incorrect to me.
>>>>>>  L8: "written by" appears twice.
>>>>>>  L11: "defautl" -> "default"
>>>>>>
>>>>> Fixed.
>>>>>> test/lib/testlibrary/jdk/testlibrary/PortFinder.java
>>>>>>  This should be coordinated with the review for 8022229 which
>>>>>> contains the same code in a different testlibrary class:
>>>>>> http://cr.openjdk.java.net/~ykantser/8022229/webrev.01/test/lib/testlibrary/jdk/testlibrary/Utils.java.html
>>>>>> <http://cr.openjdk.java.net/%7Eykantser/8022229/webrev.01/test/lib/testlibrary/jdk/testlibrary/Utils.java.html>
>>>>>>
>>>>> I made a private copy of this function, and have marked it as
>>>>> deprecated.
>>>>> As soon as ykanster's code is merged I can delete my private function
>>>>> and use the library.
>>>>> The reason for this is to get this patch pushed without further delay.
>>>>>
>>>>>> test/sun/management/jdp/7169888/JdpClient.java
>>>>>> test/sun/management/jdp/7169888/JdpDoSomething.java
>>>>>> test/sun/management/jdp/7169888/JdpTest.sh
>>>>>> test/sun/management/jdp/7169888/JdpUnitTest.java
>>>>>>  These were moved to a subdirectory, but these tests have nothing
>>>>>> to do with bug 7169888, so why that name? In any case we should
>>>>>> avoid naming test directories after bug ids, and instead use a
>>>>>> descriptive name.
>>>>>>
>>>>> This bug number was wrong. These tests were now moved back to the jdp
>>>>> folder.
>>>>> I have expanded the README file with some details about these tests.
>>>>>
>>>>> Dmitry's shell-based are as follows:
>>>>> test_01 - basic test, check if JDP packet assembler and other
>>>>>           parts of JDP is not broken
>>>>>
>>>>> test_02 - test if JDP starts with custom parameters.
>>>>>
>>>>> test_03 - test if jcmd is able to start jdp with
>>>>>           custom parameters (disabled)
>>>>>
>>>>> test_04 - test if JDP starts with default parameters
>>>>>
>>>>> test_05 - test if jcmd is able to start jdp with default
>>>>>           parameters (disabled)
>>>>>
>>>>>
>>>>> test_03 and test_05 are diabled becuase there is a mismatch between
>>>>> hotsport and jdk repos, according to Dmitry. These cases (jcmd) are
>>>>> not covered by this patch, and are part of the test enhacements [4].
>>>>>
>>>>> test_01 does partly overlap with the Java-based tests, but we should
>>>>> keep it until we implement the latest enhacements [4].
>>>>>
>>>>>
>>>>>> test/sun/management/jdp/ClientConnection.java
>>>>>>  No comments.
>>>>>>
>>>>>> test/sun/management/jdp/DefaultLauncher.java
>>>>>>  Can "@library ../../../lib/testlibrary" be replaced by "@library
>>>>>> /lib/testlibrary" ?
>>>>>>
>>>>> Fixed.
>>>>>> test/sun/management/jdp/OffLauncher.java
>>>>>>  Can "@library ../../../lib/testlibrary" be replaced by "@library
>>>>>> /lib/testlibrary" ?
>>>>>>
>>>>> Fixed.
>>>>>
>>>>>> test/sun/management/jdp/SpecificJdpAddressLauncher.java
>>>>>>  Can "@library ../../../lib/testlibrary" be replaced by "@library
>>>>>> /lib/testlibrary" ?
>>>>>>
>>>>> Fixed.
>>>>>> test/sun/management/jdp/DynamicLauncher.java
>>>>>>  No comments.
>>>>>>
>>>>> Fixed.
>>>>>
>>>>>> test/sun/management/jdp/JdpOffTest.java
>>>>>>  For the future: it is customary that the file containing the jtreg
>>>>>> directives be named xxxTest.java, and supporting files not ending in
>>>>>> Test.
>>>>>>
>>>>> I have renamed the tests following your recomendation.
>>>>> Jtreg tests have now the test suffix.
>>>>>
>>>>>> test/sun/management/jdp/JdpOnTest.java
>>>>>>  For the future: it is customary that the file containing the jtreg
>>>>>> directives be named xxxTest.java, and supporting files not ending in
>>>>>> Test.
>>>>>>
>>>>> Same as above.
>>>>>> test/sun/management/jdp/JdpTest.java
>>>>>>  No comments.
>>>>>>
>>>>>> test/sun/management/jdp/JdpTestUtil.java
>>>>>>  No comments.
>>>>>>
>>>>>> test/sun/management/jdp/JdpTestUtilTest.java
>>>>>> test/sun/management/jdp/PacketTest.java
>>>>>> test/sun/management/jdp/PortAlreadyInUseTest.java
>>>>>>  These are tests for the tests. But where are the tests for the
>>>>>> tests for the tests? :)
>>>>>>  Not reviewed.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> /Staffan
>>>>>>
>>>>>
>>>>
>>>
>>> --
>>> Alex Schenkman
>>> Java VM SQE Stockholm
>