On 9/10/2013 9:31 PM, Jaroslav Bachorik wrote:
On 9.10.2013 12:23, David Holmes wrote:
Jaroslav,
Thanks for the details description of changes - much appreciated.
There is a lot to digest in there. :)
Yep, it started as a simple fix :/
It isn't obvious to me why these tests require a full JDK?
IDK, LocalManagementTest.sh was listed as one requiring full JDK. Its
requirements are the same as the ones of CustomLauncherTest.sh (now
*.java) so it seemed logical to list it there too.
Ah! Now I see it - it uses tools.jar which implies a full JDK.
I don't quite follow the libjvm lookup logic - I would expect that you
would always want to test the libjvm that is currently running - though
it is hard to determine that.
I'm afraid I can't be of much assistance here - I just took what was in
the *.sh version and converted it to *.java.
Okay. I expect this will need revisiting at some point.
Thanks,
David
-----
-JB-
Thanks,
David
On 8/10/2013 9:33 PM, Jaroslav Bachorik wrote:
On 8.10.2013 05:42, David Holmes wrote:
Jaroslav,
Can you summarise the changes please? With the conversion to Java and
the infrastructure additions I can't tell what is actually fixing the
original timeout issue :)
The timeout was most caused by using the same file for communication
between java processes in more test cases. When those test cases were
run in parallel the file got rewritten silently and some of the tests
could end up trying to connect to incorrect port in the target
application. I was able to reproduce the timeout by interleaving the
test runs for CustomLauncherTest.sh and LocalManagementTest.sh and
adding an artificial delay to CusteomLauncherTest.sh to allow
LocalManagementTest.sh to change the port in the file.
While it could be fixed by using a different file for each test case I
took the liberty of converting the shell tests to java tests. This
allows me to remove the communication file and, in the end, make the
tests more robust.
CustomLauncherTest.java and LocalManagementTest.java are the tests
converted from shell to java. I decided to convert
LocalManagementTest.sh as well because it has the same problems as the
CustomLauncherTest.sh.
The changes in the testlibrary are about introducing new methods
allowing the tests easily start a process and wait for a certain text
appearing in its stdout/stderr. Using these methods the caller can wait
till the callee is fully initialized and eg. ready to accept
connections.
The changes in launchers make the launchers actually executable + I am
adding a linux-amd64 launcher (I needed that one to work on the changes
locally and thought it might be nice to have one more platform covered
by the test).
I've update the webrev to include changes to LocalManagementTest and
TEST.groups (both of those tests require JDK) -
http://cr.openjdk.java.net/~jbachorik/8004926/webrev.05
-JB-
Thanks,
David
On 8/10/2013 12:14 AM, Jaroslav Bachorik wrote:
On 19.9.2013 16:33, Jaroslav Bachorik wrote:
The updated webrev:
http://cr.openjdk.java.net/~jbachorik/8004926/webrev.03
I've moved some of the functionality to the testlibrary.
-JB -
On 12.9.2013 17:31, Jaroslav Bachorik wrote:
On 09/12/2013 05:13 PM, Dmitry Samersoff wrote:
Jaroslav,
CustomLauncherTest.java:
102: this check could be moved to switch at ll. 108
otherwise test fails on "sunos" and "linux" because PLATFORM
remains
unset.
Good idea. Thanks.
129: I would prefer don't have pattern like this one ever in shell
script. Could you prepare a list of VM's to check and just loop
over
it?
It makes test better readable. Also I think nowdays we can always
use
server VM.
I tried to mirror the original shell test as closely as possible. It
would be nice if we could rely on the "server" vm only. Definitely
more
readable.
-JB-
-Dmitry
On 2013-09-12 18:17, Jaroslav Bachorik wrote:
On 09/12/2013 10:22 AM, Jaroslav Bachorik wrote:
On 09/12/2013 10:12 AM, Chris Hegarty wrote:
On 09/12/2013 04:45 AM, David Holmes wrote:
Hi Jaroslav,
You need a copyright notice in the new file.
As written this test can only run on a full JDK - so please add
it to
the :needs_jdk group in TEST.groups. (Does jcmd really needs to
come
from the test-jdk? And use the VMOPTS passed to the test?)
Is there a reason this test can't run on OSX? I know it would
need
further modification but was wondering if there is something
inherent in
the test that makes it inapplicable to OSX.
I think the test would be a lot simpler if the jdk tests had
the
hotspot
test library's process tools available. :(
We have some, is there an obvious gap?
http://hg.openjdk.java.net/jdk8/tl/jdk/file/e407df8093dc/test/lib/testlibrary/jdk/testlibrary/
Hm, thanks for the info. I should have used this library instead.
Please, stand by for the updated webrev.
I was able to get rid off the JCMD. Using the testlibrary the
target
application can recognize its own PID and print it to its stdout.
The
main application then just reads the stdout to parse the PID. No
need
for JCMD any more.
I could not find a way to remove the dependency on "test.jdk"
system
property. According to the jtreg web documentation
(http://openjdk.java.net/jtreg/vmoptions.html#cmdLineOpts) a
"test.java"
system property should be available but in fact is not. But it
seems
that the testlibrary uses "test.jdk" system property too.
The test does not run on OSX because nobody built the launcher
binary :)
I think it is a kind of DIY so I took the liberty of adding a
linux-amd64 launcher while working on the test.
While working with the test library I realized I was missing a
crucial
feature (at least for my purposes) - waiting for a certain
message to
appear in the stdout/stderr of the launched process. Very often I
need
to wait for the target process to get to certain point before the
test
can be allowed to continue - and the point is indicated by a
message in
stdout/stderr. Currently all the proc tools are designed to
work in
"batch" mode - the whole stdout/stderr is captured in strings and
analyzed after the target process died - and are not suitable for
this
kind of usage.
Webrev: http://cr.openjdk.java.net/~jbachorik/8004926/webrev.01
-JB-
-Chris.
David
-----
On 12/09/2013 1:39 AM, Jaroslav Bachorik wrote:
Please, review the patch for an intermittently failing test.
The test is a shell test, using files for the interprocess
synchronization. This leads to intermittent failures.
In order to fix this the test is rewritten in Java - the
original
functionality and outputs should be 100% preserved. The
patch is
unfortunately a bit difficult to follow since there is no
similarity
between the *.sh and *.java file so one needs to go through
the
new
source in whole.
The changes in "launcher" files are all about adding
permissions to
execute (0755) and as such the webrev shows no differences.
Thanks,
Issue : JDK-8004926
Webrev :
http://cr.openjdk.java.net/~jbachorik/8004926/webrev.00
-JB-
