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.schenk...@oracle.com
<mailto:alex.schenk...@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.samers...@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
