RFR(S) : 8156681 : Add jtreg wrapper for hotspot gtest tests

[Erik Helin]

On 2016-05-13, Igor Ignatyev wrote:
> Hey Erik,
> 
> I've updated the fix to be more readable.
> 
> webrev: http://cr.openjdk.java.net/~iignatyev/8156681/webrev.02/

Looks good, Reviewed.

Thanks,
Erik

> diff b/w webrev.02 and webrev.01 : http://cr.openjdk.java.net/~iignatyev/8156681-diif/webrev.01/
> 
> — Igor
> 
> > On May 13, 2016, at 10:36 AM, Erik Helin <erik.helin at oracle.com> wrote:
> > 
> > On 2016-05-12, Igor Ignatyev wrote:
> >> Jesper,
> >> 
> >> 1st of all, thank you for review.
> >> 
> >> unfortunately, testing in our infra reveled some problems w/ the wrapper:
> >> - gtest test library can be located right in ‘test.nativepath’ or in ‘test.nativepath’/../../gtest
> >> - the wrapper pass all flags to gtestLauncher including flags which are not JVM flags, gtestLauncher pass all these args to JNI_CreateJavaVM, so we fail w/ 'Unrecognized option'
> >> 
> >> I have updated the webrev : http://cr.openjdk.java.net/~iignatyev/8156681/webrev.01/
> > 
> > I don't understand the first path. Is System.getProperty("test.nativepath") either
> > "<test_image>/hotspot/jtreg/native" or "<test_image>/hotspot/gtest"? Or
> > is it something else?  What do you want the final path to be?
> the final path should be <test_image>/hotspot/gtest/<vm_variant>
> > 
> > 
> > Is the indentation a bit off here?
> >  62                 Stream.of(
> >  63                         path.toString(),
> >  64                         "-jdk",
> >  65                         System.getProperty("test.jdk")),
> >  66                 Arrays.stream(Utils.getTestJavaOpts())
> >  67                       .filter(s -> s.startsWith("-X") || s.startsWith("-D")))
> > 
> > since you always want the first three arguments, is the following a bit easier to understand?
> I split that into 3 statements.
> > 
> >  List<String> options = Arrays.stream(Utils.getTestJavaOpts())
> >                               .filter(s -> s.startsWith("-X") || s.startsWith("-D"))
> >                               .collect(Collectors.toList());
> >  List<String> cmd = Arrays.asList(path.toString(), "-jdk", System.getProperty("test.jdk"));
> >  cmd.addAll(options);
> > 
> > The following
> >  70         OutputAnalyzer oa = ProcessTools.executeProcess(builder);
> >  71         oa.shouldHaveExitValue(0);
> > 
> > could also be shortened to one line if you prefer that:
> > 
> >  ProcessTools.executeProcess(builder).shouldHaveExitValue(0);
> > 
> > Thanks,
> > Erik
> > 
> >> 
> >> could you please review the updated version?
> >> 
> >> Thanks,
> >> — Igor 
> >> 
> >>> On May 10, 2016, at 8:32 PM, Jesper Wilhelmsson <jesper.wilhelmsson at oracle.com> wrote:
> >>> 
> >>> Looks good!
> >>> /Jesper
> >>> 
> >>> Den 10/5/16 kl. 18:31, skrev Igor Ignatyev:
> >>>> http://cr.openjdk.java.net/~iignatyev/8156681/webrev.00/
> >>>>> 71 lines changed: 71 ins; 0 del; 0 mod;
> >>>> 
> >>>> Hi all,
> >>>> 
> >>>> could you please integrate this small fix which adds a jtreg wrapper to run gtest tests?
> >>>> 
> >>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8156681
> >>>> webrev: http://cr.openjdk.java.net/~iignatyev/8156681/webrev.00/
> >>>> 
> >>>> Thanks,
> >>>> — Igor
> >>>> 
> >> 
>