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

[Staffan Larsen]

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> 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> wrote:
>> 
>>> Staffan, thanks for your review!
>>> 
>>> The new webrev can be found here: http://cr.openjdk.java.net/~ykantser/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> wrote:
>>>> 
>>>>> Hi,
>>>>> 
>>>>> Could I please have a review of this fix.
>>>>> 
>>>>> webrev: http://cr.openjdk.java.net/~ykantser/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/15a15475/attachment.html>