RFR(S) : 8156681 : Add jtreg wrapper for hotspot gtest tests
Erik Helin
erik.helin at oracle.com
Fri May 13 07:36:07 UTC 2016
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?
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?
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
> >>
>
More information about the hotspot-dev
mailing list