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

[Alex Schenkman]

The latest version is available here [1].

Changes:
1. All of Staffan's comments (below) have been addressed.
2. DynamicLauncher 
<http://cr.openjdk.java.net/%7Edsamersoff/sponsorship/aschenkman/8014506/webrev.02/test/sun/management/jdp/DynamicLauncher.java.html> 
will retry 3 times before giving up on a busy jmx port.
3. Why the mix of standard jtreg tests and TestNG tests?
TestNG tests are meant to be used during test development; they are 
tests for the tests.
They should not be run automatically by JTreg, but manually by the test 
developer.

Thanks!

[1] 
http://cr.openjdk.java.net/~dsamersoff/sponsorship/aschenkman/8014506/webrev.02/ 
<http://cr.openjdk.java.net/%7Edsamersoff/sponsorship/aschenkman/8014506/webrev.02/>


On 2013-08-21 18:15, Staffan Larsen wrote:
> Alex,
>
> This looks a lot better than the previous version. Thanks for cleaning 
> things up. Below are a couple of comments:
>
> - Use of "package" is discouraged for jtreg test: 
> http://openjdk.java.net/jtreg/faq.html#question4.9
>
> - Maybe change name of SpecificLauncher.java to 
> SpecificJdpAddressLauncher.java?
>
> - The constructor and onSocketTimeOut() in JdpOffTest.java has wrong 
> indentation.
>
> - The comment for JdpOffTest.shouldContinue() has weird indentation.
>
> - How does one run PacketTest.java? It looks like a TestNG test, but I 
> couldn't get jtreg to run it.
>
> - Why the mix of standard jtreg tests and TestNG tests?
>
> - nit: somewhat non-standard formatting with spaces inside 
> parens: socket.receive( datagram );
>
> - nit: statements should be on a new line:
>             if ( hasTestLivedLongEnough() ) { shutdown(); }
>
>
> Thanks,
> /Staffan
>
> On 21 aug 2013, at 14:01, Alex Schenkman <alex.schenkman at oracle.com 
> <mailto:alex.schenkman at oracle.com>> wrote:
>
>> Staffan, do you think you can take another look this?
>> Thanks!
>>
>> On 2013-07-04 14:26, Alex Schenkman wrote:
>>> Thanks Staffan for your comments.
>>>
>>> The indentation is corrected in all files, sorry about that.
>>>
>>> About the racing condition in PortFinder.java, you are absolutely 
>>> right, there is no warranty that this port will be free.
>>> I do not know a better way to do this. Any suggestions?
>>> On the other hand, even if this fails every now and then, it will 
>>> work most of the times and it is better than what we have today.
>>> I have added better comments in PortFinder to reflect the problem.
>>>
>>> The new webrev is here [1], at the same address than before.
>>>
>>> Thanks!
>>>
>>> [1] http://cr.openjdk.java.net/~dsamersoff/8014506/webrev.01/ 
>>> <http://cr.openjdk.java.net/%7Edsamersoff/8014506/webrev.01/>
>>>
>>> PS:
>>> PortFinder.java, line 37:
>>> There is an extra <p/> tag that should be removed.
>>>
>>>
>>>
>>> On 2013-06-26 13:54, Staffan Larsen wrote:
>>>> test/lib/testlibrary/jdk/testlibrary/PortFinder.java:
>>>>
>>>> - This looks racy to me. There is no guarantee that the port will still be available after the call to close().
>>>> - Missing copyright notice.
>>>> - In comment "on of them" -> "one of them"
>>>>
>>>> test/sun/management/jdp/ClientConnection.java:
>>>>
>>>> - broken indentation
>>>> - inconsistent spacing in method calls. Sometime "joinGroup(address);" and sometimes "setSoTimeout( msTimeOut );". Should be without spaces.
>>>>
>>>> test/sun/management/jdp/DynamicLauncher.java:
>>>>
>>>> - type: binded -> bound
>>>> - inconsistent spacing in method calls
>>>>
>>>> test/sun/management/jdp/JdpOffTest.java
>>>>
>>>> - broken indentation
>>>>
>>>> test/sun/management/jdp/JdpTestUtil.java
>>>>
>>>> - broken indentation
>>>>
>>>>
>>>> I stopped reading here. Can we get these basic things fixed before the next review round, please?
>>>>
>>>> /Staffan
>>>>
>>>>
>>>> On 26 jun 2013, at 12:21, Dmitry Samersoff<Dmitry.Samersoff at oracle.com>  wrote:
>>>>
>>>>> Nils,
>>>>>
>>>>> I'm sponsoring this push but don't see any review replies in this thread.
>>>>>
>>>>> Webrev refreshed against latest tl is here:
>>>>>
>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8014506/webrev.01/
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>> On 2013-05-27 12:25, Alex Schenkman wrote:
>>>>>> Please review these tests for the Jdp protocol.
>>>>>>
>>>>>> There are three cases:
>>>>>> 1) Jdp is off.
>>>>>> 2) Jdp is on and using default address and port.
>>>>>> 3) Jdp is on using custom address and port.
>>>>>>
>>>>>> Tests:http://cr.openjdk.java.net/~nloodin/8014506/webrev.00/
>>>>>> <http://cr.openjdk.java.net/%7Enloodin/8014506/webrev.00/>
>>>>>> Jdp feature enhancement:https://jbs.oracle.com/bugs/browse/JDK-8002048
>>>>>> JDK CCC:http://ccc.us.oracle.com/8002048
>>>>>>
>>>>>>
>>>>>> Thanks in advance!
>>>>>>
>>>>>> -- 
>>>>>> Alex Schenkman
>>>>>> Java VM SQE Stockholm
>>>>>>
>>>>> -- 
>>>>> Dmitry Samersoff
>>>>> Oracle Java development team, Saint Petersburg, Russia
>>>>> * I would love to change the world, but they won't give me the sources.
>>>
>>> -- 
>>> Alex Schenkman
>>> Java VM SQE Stockholm
>>
>> -- 
>> Alex Schenkman
>> Java VM SQE Stockholm
>

-- 
Alex Schenkman
Java VM SQE Stockholm

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130828/6920b821/attachment.html