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

[Alex Schenkman]

I've added a README file and moved Dmitry's tests into its own folder, 
to address the comments below.
http://cr.openjdk.java.net/~dsamersoff/sponsorship/aschenkman/8014506/webrev.03/ 
<http://cr.openjdk.java.net/%7Edsamersoff/sponsorship/aschenkman/8014506/webrev.03/>

On 2013-08-21 20:10, Staffan Larsen wrote:
> A couple of more comments:
>
> - It was a little too hard to figure out exactly how the tests worked: 
> who is sending JDP packets? who is listening for them? who is 
> launching the tests? Perhaps it would make sense to add a small readme 
> which explains how it all fits together?
>
> - There are already some JDP tests in that directory which are also 
> made up of a couple of files. Mixing them together makes it hard to 
> see which files belong to which set of tests. Maybe we should have 
> distinct subfolders for the sets?
>
> - There is also the question of how much overlap there are between 
> your tests and the already existing tests. It would be preferable to 
> have no overlap. Perhaps some of the existing shell-based tests can be 
> subsumed by your java-based tests?
>
> Thanks,
> /Staffan
>
> On 21 aug 2013, at 18:15, Staffan Larsen <staffan.larsen at oracle.com 
> <mailto:staffan.larsen at oracle.com>> 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/20130902/67e079e7/attachment.html