RFR (S): 8026043: add regression test for JDK-8000831

[Filipp Zhinkin]

Erik,

thanks for review!

Are there any other volunteers to review it? :)

Filipp.

On 11/26/2013 02:32 PM, Erik Helin wrote:
> Filipp,
>
> this looks good, thanks for the test!
>
> Erik
>
> On 2013-11-26 10:48, Filipp Zhinkin wrote:
>> Eric,
>>
>> thanks for looking at it!
>>
>> I've fixed all issues that you've listed below.
>> Here is updated webrev:
>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.03/
>>
>> Thanks,
>> Filipp.
>>
>> On 11/25/2013 06:00 PM, Erik Helin wrote:
>>> Filipp,
>>>
>>> On 2013-11-06 17:11, Filipp Zhinkin wrote:
>>>> Hi,
>>>>
>>>> may I have a review on it?
>>>
>>> Overall, the test looks fine. Only three small comments:
>>> - Could you @run main/samevm? I think that you can actually remove @run
>>>   tag, since AFAIK the default action for jtreg is:
>>>        @run main/samevm NameOfTest
>>> - The method ProcessTools.createVerboseJavaProcessBuilder is not
>>>   available yet (it is in one of your other changes), can you use the
>>>   existing ProcessTools.createJavaProcessBuilder for now?
>>> - Could you make the garbage variable public?
>>>
>>> Thanks for writing new tests!
>>> Erik
>>>
>>>> Thanks,
>>>> Filipp.
>>>>
>>>> On 10/22/2013 12:40 PM, Filipp Zhinkin wrote:
>>>>> Hi,
>>>>>
>>>>> I've done following changes:
>>>>> - @build tag removed;
>>>>> - 'garbage' array is now static field;
>>>>> - added heap-sizing flags to avoid failures with options passed via
>>>>> 'test.java.opts'.
>>>>>
>>>>> Updated webrev: 
>>>>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.02/
>>>>>
>>>>> Thanks,
>>>>> Filipp.
>>>>>
>>>>> On 10/18/2013 07:33 PM, Filipp Zhinkin wrote:
>>>>>> Hi Erik,
>>>>>>
>>>>>> thank you for watching at it!
>>>>>>
>>>>>> On 10/18/2013 06:59 PM, Erik Helin wrote:
>>>>>>> Hi Filipp,
>>>>>>>
>>>>>>> thanks for adding new tests, really appreciate it!
>>>>>>>
>>>>>>> I have a couple of comments/questions about this test:
>>>>>>> - Do you really need a @build tag?
>>>>>> No, I don't.
>>>>>>> - Could you please provide a comment above each regex with an 
>>>>>>> example
>>>>>>>    that shows what the regex matches?
>>>>>> Yes, sure.
>>>>>>> - This test will only run with the default GC (since no GC is
>>>>>>>    specified). What do you think about additional run targets 
>>>>>>> testing
>>>>>>>    various GC combinations?
>>>>>> The idea was to pass all VM options including different GC's via
>>>>>> test.java.opts property.
>>>>>> It makes test more flexible and allow us to test not only some fixed
>>>>>> subset of GC combinations.
>>>>>>> - Have you tried running the test with -Xcomp? The reason I'm
>>>>>>> asking is
>>>>>>>    because I'm afraid that the compiler might be able to realize
>>>>>>> that the
>>>>>>>    GarbageProducer main method isn't doing anything.
>>>>>> Yes, I've tried and that object allocation was not optimized out,
>>>>>> but I agree with you, "garbage" should be static.
>>>>>>>
>>>>>>>    One thing that usually solves this is having garbage as a public
>>>>>>>    static variable in the class.
>>>>>>> - Are you using GarbageProducer since you want to trigger young
>>>>>>> GCs? If
>>>>>>>    so, why only tests young GCs? Would the test works just as well
>>>>>>> using
>>>>>>>    System.gc() to tests full GCs?
>>>>>> In fact, test running with MaxHeapSize=5m, so both minor and full GC
>>>>>> happens.
>>>>>> But there is another issue then  - I forgot to set up other heap
>>>>>> sizing options,
>>>>>> so test will fail when someone start if with, for example, -Xms10m.
>>>>>>
>>>>>> I'll fix that.
>>>>>>
>>>>>> Thanks,
>>>>>> Filipp.
>>>>>>> Thanks,
>>>>>>> Erik
>>>>>>>
>>>>>>> On 2013-10-14, Filipp Zhinkin wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I've refactored options parsing, here is updated webrev:
>>>>>>>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.01/
>>>>>>>>
>>>>>>>> Note that I'm using ProcessTools.createVerboseJavaProcessBuilder
>>>>>>>> from 8026047
>>>>>>>> to make sure that VM output will not be suppressed or 
>>>>>>>> redirected to
>>>>>>>> file.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Filipp.
>>>>>>>>
>>>>>>>> On 10/11/2013 08:22 PM, Tao Mao wrote:
>>>>>>>>> Ok, thank you for heads-up. Then, I'll await your webrev update.
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>>>>> Tao
>>>>>>>>>
>>>>>>>>> On 10/11/13 9:11 AM, Filipp Zhinkin wrote:
>>>>>>>>>> Hi Tao,
>>>>>>>>>>
>>>>>>>>>> Sorry for typo. You're right, the correct one is
>>>>>>>>>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.00/.
>>>>>>>>>>
>>>>>>>>>> And I'll refactor it in the same way as fix for 8026047.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Filipp.
>>>>>>>>>>
>>>>>>>>>> On 10/11/2013 08:11 PM, Tao Mao wrote:
>>>>>>>>>>> Hi Filipp,
>>>>>>>>>>>
>>>>>>>>>>> FYI, it has a wrong webrev. I guess it should be
>>>>>>>>>>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> Thanks.
>>>>>>>>>>> Tao
>>>>>>>>>>>
>>>>>>>>>>> On 10/8/13 8:34 AM, Filipp Zhinkin wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I would like to get couple review on test, that cover issue
>>>>>>>>>>>> fixed by 8000831 - Heap verification output
>>>>>>>>>>>> incorrect/incomplete.
>>>>>>>>>>>>
>>>>>>>>>>>> Test verifies that options VerifyBeforeGC and
>>>>>>>>>>>> VerifyAfterGCemit appropriate output if they are turned on
>>>>>>>>>>>> and never
>>>>>>>>>>>> emit corrupted output as it was before 8000831 fixed.
>>>>>>>>>>>>
>>>>>>>>>>>> Bug ID: https://bugs.openjdk.java.net/browse/JDK-8026043
>>>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~kshefov/8026047/webrev.00/
>>>>>>>>>>>> Testing: manual on local host, automated on various platforms.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Filipp.
>>>>>>
>>>>>
>>>>
>>