RFR(S): 6545295: TEST BUG: The test HatHeapDump1Test uses wrong path in exec call.

[Yekaterina Kantserova]

Oh, great! Could you please be my sponsor? The patch is attached to this 
mail.

// Katja



On 06/16/2014 10:45 AM, Staffan Larsen wrote:
> Katja,
>
> The policy for reviews in the jdk repo is different from the hotspot 
> repo. For the jdk it is ok with a single review.
>
> /Staffan
>
>
> On 16 jun 2014, at 09:59, Yekaterina Kantserova 
> <yekaterina.kantserova at oracle.com 
> <mailto:yekaterina.kantserova at oracle.com>> wrote:
>
>> Hi,
>>
>> could I get one more review please?
>>
>> Thanks,
>> Katja
>>
>>
>> On 06/13/2014 02:23 PM, Staffan Larsen wrote:
>>> Looks good!
>>>
>>> Thanks,
>>> /Staffan
>>>
>>> On 13 jun 2014, at 14:15, Yekaterina Kantserova 
>>> <yekaterina.kantserova at oracle.com 
>>> <mailto:yekaterina.kantserova at oracle.com>> wrote:
>>>
>>>> Staffan, thanks for your review!
>>>>
>>>> The new webrev can be found 
>>>> here:http://cr.openjdk.java.net/~ykantser/6545295/webrev.01/ 
>>>> <http://cr.openjdk.java.net/%7Eykantser/6545295/webrev.01/>
>>>>
>>>> // Katja
>>>>
>>>> On 06/13/2014 01:44 PM, Staffan Larsen wrote:
>>>>> Katja,
>>>>>
>>>>> Great to see this simplification! A couple of comments:
>>>>>
>>>>> - I don’t think you need to launch jhat with -XX:+UsePerfData - no 
>>>>> one is attaching or reading data from the process.
>>>>>
>>>>> - The static processBuilder field seems to be mis-used. Why not 
>>>>> use different ProcessBuilder objects for the HelloWorld and jhat?
>>>>>
>>>>> - For dumpFile.deleteOnExit() to be useful you should add it as 
>>>>> early as possible (line 48 perhaps?). If it is the last line in 
>>>>> the test you can just as well do dumpFile.delete().
>>>>
>>>> Of cause it should be dumpFile.delete()! - dumpFile.deleteOnExit() 
>>>> has crept in by mistake.
>>>>
>>>>>
>>>>> /S
>>>>>
>>>>>
>>>>> On 13 jun 2014, at 13:25, Yekaterina Kantserova 
>>>>> <yekaterina.kantserova at oracle.com 
>>>>> <mailto:yekaterina.kantserova at oracle.com>> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Could I please have a review of this fix.
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ykantser/6545295 
>>>>>> <http://cr.openjdk.java.net/%7Eykantser/6545295>
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-6545295
>>>>>>
>>>>>> The test has been re-written using the testlibrary's 
>>>>>> functionality. It's verified internally on all basic platforms.
>>>>>>
>>>>>> Thanks,
>>>>>> Katja
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140616/f7e40cca/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 6545295.1.patch.zip
Type: application/zip
Size: 3871 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140616/f7e40cca/6545295.1.patch-0001.zip>