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

[Mikael Auno]

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>> 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
>