jmx-dev [ping][ping] Re: RFR: 8004926 sun/management/jmxremote/bootstrap/CustomLauncherTest.sh oftenly times out

[Jaroslav Bachorik]

On 7.10.2013 16:31, Dmitry Samersoff wrote:
> Jarsolav,
>
> Looks good for me, comments below is just a nits - so fill free to
> ignore it.
>
> 1.
> As FS.getPath(TEST_JDK, "jre", "lib", LIBARCH) is the only value for
> findLibjvm parameter, It's better to create an overload function
> findLibjvm().
>
> 2.
> it's better to check for File.isFile() - readable (e.g. device) is not
> always what you whant here.
>
> 3. It's good to try
> ARCH/libjvm.so, ARCH/server/libjvm.so, ARCH/client/libjvm.so
> in order for the possible platforms with the only vm

Nits not ignored - 
http://cr.openjdk.java.net/~jbachorik/8004926/webrev.04/ :)

-JB-

>
> -Dmitry
>
>
> On 2013-10-07 18:14, 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-
>>>>>>>>>>
>>>>>
>>>
>>
>
>