Request for review: 7104173 sun/tools tests fail with debug build after 7012206

[Paul Hohensee]

Looks good to me too.  And, I'm an "official reviewer". :)

Paul

On 10/27/11 8:00 AM, Coleen Phillimore wrote:
> This still seems fine to me (after reading the discussion and 
> 6605954).  I don't know why someone wanted PrintVMOptions on in the 
> regular product, but it having this divergence for embedded is okay 
> with me.
>
> Coleen
>
> On 10/27/2011 6:47 AM, David Holmes wrote:
>> On 27/10/2011 7:13 PM, Dmitry Samersoff wrote:
>>>> I don't know the history of that option but maybe we should disable
>>>> it for non-embedded builds too?
>>>
>>> +1
>>>
>>> I also would prefer to keep the same behavior for embedded/non-embedded
>>> when possible.
>>
>> 6605954 was closed as "not a defect". So non-embedded has already 
>> expressed a desire to keep the current behaviour. Hence the 
>> embedded-only change.
>>
>> Thanks for the "reviews" ;-) I'll wait and see if I can get another 
>> formal review from runtime before pushing.
>>
>> David
>>
>>> -Dmitry
>>>
>>> On 2011-10-27 13:00, Christian Thalinger wrote:
>>>>
>>>> On Oct 27, 2011, at 10:43 AM, David Holmes wrote:
>>>>
>>>>> Hi Chris,
>>>>>
>>>>> On 27/10/2011 5:51 PM, Christian Thalinger wrote:
>>>>>>
>>>>>> On Oct 26, 2011, at 8:33 AM, David Holmes wrote:
>>>>>>
>>>>>>> webrev:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dholmes/7104173/webrev/
>>>>>>>
>>>>>>> A trivial change for our embedded code to aid in testing. Some 
>>>>>>> tests need to be passed -XX:+UsePerfData to make them work 
>>>>>>> (7012206). A debug build, by default, prints any -XX options 
>>>>>>> passed to it. Any such tests that parse the test output then 
>>>>>>> fail with a debug build because of this extra unexpected output.
>>>>>>>
>>>>>>> So for embedded only we simply turn PrintVMOptions off by 
>>>>>>> default for all builds.
>>>>>>
>>>>>> Why aren't you also passing -PrintVMOptions for the tests that 
>>>>>> need +UsePerfData?
>>>>>
>>>>> When 701226 was fixed I didn't realize there would be a need to 
>>>>> add -PrintVMOptions. Rather than go through and pollute all those 
>>>>> test scripts with yet-another-option I decided to deal with the 
>>>>> problem at the source - the VM. There really is no need to print 
>>>>> the options by default with a debug build. So for the embedded 
>>>>> build we turn it off. (See also 6605954)
>>>>
>>>> I don't know the history of that option but maybe we should disable 
>>>> it for non-embedded builds too?  Sometimes the output of the VM 
>>>> options is useful but in that case I can add +PrintVMOptions manually.
>>>>
>>>> I don't have a strong opinion here I just don't like that embedded 
>>>> and non-embedded are diverging here.  Anyway, the change is 
>>>> basically okay :-)
>>>>
>>>> -- Chris
>>>>
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>
>>>>>> -- Chris
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>
>>>>
>>>
>>>