Jaroslav, I'm not sure that replacing 50 lines shell script with 250 lines Java program we should keep it close to original. ;)
-Dmitry On 2013-09-12 19: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- >>>>>>> >>>> >>> >> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
