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

[Yekaterina Kantserova]

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/fffea664/attachment.html>