RFR(XS): 8251121: six SA tests leave core files behind on macOS

[David Holmes]

Update looks good.

Thanks,
David

On 7/08/2020 7:50 am, Chris Plummer wrote:
> Hi David and Dan,
> 
> I went with just logging how long the copy takes. Here's all the code 
> involved:
> 
>              if (corePath.getParent() != null) {
>                  Path coreFileName = corePath.getFileName();
>                  System.out.println("Moving core file to cwd: " + 
> coreFileName);
>                  long startTime = System.currentTimeMillis();
>                  Files.move(corePath, coreFileName);
>                  System.out.println("Core file move took " + 
> (System.currentTimeMillis() - startTime) + "ms");
>                  coreFileLocation = coreFileName.toString();
>              }
> 
> On linux where it didn't actually end up having to move the file (src 
> and dest paths are the same), it reported 0ms. On OSX where it did move 
> the file from /cores, it reported 2ms.
> 
> Let me know if you're ok with these changes.
> 
> thanks,
> 
> Chris
> 
> On 8/6/20 11:31 AM, Chris Plummer wrote:
>> Hi Dan,
>>
>> On 8/6/20 10:17 AM, Daniel D. Daugherty wrote:
>>> On 8/5/20 9:16 PM, Chris Plummer wrote:
>>>> Hello,
>>>>
>>>> Please review the following:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8251121
>>>> http://cr.openjdk.java.net/~cjplummer/8251121/webrev.00/index.html
>>>
>>> test/lib/jdk/test/lib/util/CoreUtils.java
>>>     You might consider two messages with timestamps: one before the move
>>>     and one after the move completes.
>>>
>> Do we have an standard timestamp printing support for our jtreg tests? 
>> I found the following in vmTestbase/nsk/share/Log.java:
>>
>>     /**
>>      * Compose line to print possible prefixing it with timestamp.
>>      */
>>     private String composeLine(String message) {
>>         if (timestamp) {
>>             long time = System.currentTimeMillis();
>>             long ms = time % 1000;
>>             time /= 1000;
>>             long secs = time % 60;
>>             time /= 60;
>>             long mins = time % 60;
>>             time /= 60;
>>             long hours = time % 24;
>>             return "[" + hours + ":" + mins + ":" + secs + "." + ms + 
>> "] " + message;
>>         }
>>         return message;
>>     }
>>
>> Would be nice if we had something like that more generally available 
>> to all jtreg tests.
>>
>> thanks,
>>
>> Chris
>>> test/hotspot/jtreg/serviceability/sa/ClhsdbCDSCore.java
>>>     No comments.
>>>
>>> Thumbs up. No need for another webrev if you decide to update the mesgs.
>>>
>>> I'm testing your patch on my MBP13 to verify that it solves the issue
>>> that I reported.
>>>
>>> Dan
>>>
>>>
>>>>
>>>> On OSX (and possibly some linux systems), core files are not 
>>>> produced in the cwd, but instead end up in some well known location. 
>>>> For OSX it is the /cores directory. The core files tend to 
>>>> accumulate there. This fixes the core file accumulation problem by 
>>>> moving the core file into the cwd, allowing jtreg to manage it. By 
>>>> default jtreg will delete the core if the test passes, and retain if 
>>>> if the test fails or RETAIN=all is specified.
>>>>
>>>> I got rid of the code in ClhsdbCDSCore.java that explicitly deletes 
>>>> the core file because we don't want it deleted if RETAIN=all is used.
>>>>
>>>> thanks,
>>>>
>>>> Chris
>>>
>>
>