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

2013-05-27 Thread Alex Schenkman
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/

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

2013-10-29 Thread Alex Schenkman
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

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

2013-06-26 Thread Dmitry Samersoff
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. > > T

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

2013-06-26 Thread Staffan Larsen
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 in

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

2013-07-04 Thread Alex Schenkman
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, e

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

2013-08-21 Thread Alex Schenkman
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

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

2013-08-21 Thread Staffan Larsen
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 SpecificJdpAddressLaunche

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

2013-08-21 Thread Staffan Larsen
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 so

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

2013-08-28 Thread Alex Schenkman
The latest version is available here [1]. Changes: 1. All of Staffan's comments (below) have been addressed. 2. DynamicLauncher will retry 3 times before giving

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

2013-09-02 Thread 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/ On 2013-08-21 20:10, Staffa

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

2013-10-23 Thread Alex Schenkman
Hi Staffan, Any chance you can review this again? http://cr.openjdk.java.net/~dsamersoff/sponsorship/aschenkman/8014506/ Thanks in advance! On 2013-09-02 09:56, Alex Schenkman wrote: I've added a README file and moved

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

2013-10-24 Thread Staffan Larsen
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" test/lib/testlibrary/jdk/testlibrary/PortFinder.java This should be coordinated with the review for 809 whi

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

2013-10-30 Thread Staffan Larsen
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 Ut

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

2013-11-08 Thread Alex Schenkman
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/ On 2013-10-30 16:03, Staffan Larsen wrot

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

2013-11-08 Thread 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 S

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

2013-11-11 Thread Staffan Larsen
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 wrote: > There was some unintended whitespace changes in webrev.05 that Alex had > missed. I removed them

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

2013-11-11 Thread 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 a

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

2013-11-11 Thread Staffan Larsen
Thanks for the explanation. Reviewed! I will sponsor the push. And thanks for hanging in there during the review process. /Staffan On 11 Nov 2013, at 14:57, Mikael Auno wrote: > The reduction in testsuites in JdpTest.sh is due to those "testsuites" > removed being ported to Java with this ch