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

[Jesper Wilhelmsson]

Looks good!
/Jesper

Den 13/5/16 kl. 18:09, skrev Igor Ignatyev:
> Hey Erik,
>
> I've updated the fix to be more readable.
>
> webrev: http://cr.openjdk.java.net/~iignatyev/8156681/webrev.02/
> 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
>>>>>
>>>
>