Fwd: Re: RFR: 8062536: [TESTBUG] Conflicting GC combinations in jdk tests

[Evgeniya Stepanova]

Hi David!

Ok, I'll change the diff.

Thank you
On 07.11.2014 9:37, David Holmes wrote:
> On 7/11/2014 12:36 AM, Evgeniya Stepanova wrote:
>> New webrev:
>> http://cr.openjdk.java.net/~eistepan/8062536/webrev.01/
>
> In:
>
> test/java/lang/management/RuntimeMXBean/TestInputArgument.sh
>
> the use of the gc options seems incidental - it's just picking two 
> innocuous options to use - similar to the JpsHelper case. You could 
> replace +UseParallelGC with something like +UseFastJNIAccessors 
> (platform independent and normally true).
>
> David
> -----
>
>> Thanks,
>> Evgeniya Stepanova
>> On 06.11.2014 17:35, Yekaterina Kantserova wrote:
>>> Thanks a lot!
>>>
>>> On 11/06/2014 02:05 PM, Evgeniya Stepanova wrote:
>>>> Hi Katja,
>>>>
>>>> Ok, this seems to be a perfect solution. Thank you. I'll change the
>>>> diff accordingly.
>>>>
>>>>
>>>> Thanks,
>>>> Evgeniya Stepanova
>>>> On 06.11.2014 16:56, Yekaterina Kantserova wrote:
>>>>> Hi Dima,
>>>>>
>>>>> On 11/06/2014 11:22 AM, Dmitry Fazunenko wrote:
>>>>>> Hi Katja,
>>>>>>
>>>>>> You are right, there will be no conflict, because test ignores any
>>>>>> external VM flags.
>>>>>> So, adding @requires seems unnecessary here, but...
>>>>>>
>>>>>> Ignoring external options is bad thing, such "selfish" tests are
>>>>>> not applicable for other areas, like GC, compiler, RT.
>>>>>
>>>>> This particular case is to test the defined flags are shown up as
>>>>> expected.
>>>>>
>>>>> Evgeniya,
>>>>>
>>>>> would you mind to change JpsHelper.java instead?
>>>>>
>>>>> +++ b/test/sun/tools/jps/JpsHelper.java
>>>>> @@ -93,7 +93,7 @@
>>>>>      /**
>>>>>       * VM arguments to start test application with
>>>>>       */
>>>>> -    public static final String[] VM_ARGS = {"-Xmx512m",
>>>>> "-XX:+UseParallelGC"};
>>>>> +    public static final String[] VM_ARGS = {"-Xmx512m",
>>>>> "-XX:+PrintGCDetails"};
>>>>>      /**
>>>>>       * VM flag to start test application with
>>>>>       */
>>>>>
>>>>> Best regards,
>>>>> Katja
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> @requires  will allow to modify tests to include external vm
>>>>>> options without any risk of bumping into conflict and extend area
>>>>>> of test applicability.
>>>>>>
>>>>>> But if you still believe, that @requires is not necessary - it's
>>>>>> not a problem, tests could be kept as is.
>>>>>>
>>>>>> Thanks,
>>>>>> Dima
>>>>>>
>>>>>>
>>>>>> On 06.11.2014 16:27, Yekaterina Kantserova wrote:
>>>>>>>
>>>>>>> Hi Evgeniya,
>>>>>>>
>>>>>>> As David has pointed out these jps tests are not testing gc. The
>>>>>>> -XX:+UseParallelGC is just an arbitrary chosen test flag. There
>>>>>>> should not be any conflicts either since these tests are running
>>>>>>> in driver mode:
>>>>>>>
>>>>>>> ...
>>>>>>>  * @run driver TestJpsJar
>>>>>>> ...
>>>>>>>
>>>>>>> which means no flags from above are accepted.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Katja
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 11/06/2014 11:05 AM, Evgeniya Stepanova wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> tag added because tests contain string
>>>>>>>>  cmd.addAll(JpsHelper.getVmArgs());
>>>>>>>>
>>>>>>>> and JpsHelper defines
>>>>>>>> ...
>>>>>>>> public static final String[] VM_ARGS = {"-Xmx512m",
>>>>>>>> "-XX:+UseParallelGC"};
>>>>>>>> ...
>>>>>>>> public static List<String> getVmArgs() throws IOException {
>>>>>>>>         if (testVmArgs == null) {
>>>>>>>>             testVmArgs = new ArrayList<>();
>>>>>>>> testVmArgs.addAll(Arrays.asList(VM_ARGS));
>>>>>>>>             testVmArgs.add("-XX:Flags=" +
>>>>>>>> getVmFlagsFile().getAbsolutePath());
>>>>>>>>         }
>>>>>>>>         return testVmArgs;
>>>>>>>>     }
>>>>>>>>
>>>>>>>> Tests itself wouldn't fail if we use another GC, tag added for
>>>>>>>> cleanup-if we use for example SerialGC we must be sure that tests
>>>>>>>> passed with this GC, not with another one. Now you will assume
>>>>>>>> that concrete test passed with Serial GC, but it run only with
>>>>>>>> Parallel GC. Plus there is no any sense to run test twice in TC
>>>>>>>> (with different GC, since it use only Parallel)
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Evgeniya Stepanova
>>>>>>>> On 06.11.2014 6:20, David Holmes wrote:
>>>>>>>>> Hi Evgeniya,
>>>>>>>>>
>>>>>>>>> On 6/11/2014 1:33 AM, Evgeniya Stepanova wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Please review changes for 8062536, the OpenJDK/jdk part of the
>>>>>>>>>> JDK-8019361
>>>>>>>>>>
>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8062536
>>>>>>>>>> fix: http://cr.openjdk.java.net/~eistepan/8062536/webrev.00/
>>>>>>>>>>
>>>>>>>>>> Problem: Some tests explicitly set GC and fail when another GC
>>>>>>>>>> is set
>>>>>>>>>> outside
>>>>>>>>>
>>>>>>>>> I don't see why you have done this for the
>>>>>>>>>
>>>>>>>>> test/sun/tools/jps/TestJps*.java
>>>>>>>>>
>>>>>>>>> tests. They don't set any GC flags.
>>>>>>>>>
>>>>>>>>>> Solution: Such tests marked with the jtreg tag "requires" to
>>>>>>>>>> skip test
>>>>>>>>>> if there is a conflict
>>>>>>>>>
>>>>>>>>> Just wondering: Does a skipped test get a .jtr file showing it
>>>>>>>>> was skipped; or does it only appear in the higher-level jtreg 
>>>>>>>>> log?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> Tested locally with different GC flags (-XX:+UseG1GC,
>>>>>>>>>> -XX:+UseParallelGC, -XX:+UseSerialGC, -XX:+UseConcMarkSweep and
>>>>>>>>>> without
>>>>>>>>>> any GC flag). Tests are being excluded as expected. No tests
>>>>>>>>>> failed
>>>>>>>>>> because of the conflict.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Evgeniya Stepanova
>>>>>>>>>>
>>>>>>>>>> //
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> /Evgeniya Stepanova/
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>> -- 
>>>> /Evgeniya Stepanova/
>>>
>>
>> -- 
>> /Evgeniya Stepanova/

-- 
/Evgeniya Stepanova/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20141107/404cdb6c/attachment-0001.html>