RFR: 8203491: [TESTBUG] Port heapdump tests into java

[Jini George]

Hi Leonid,

I have mostly looked at only the following file:

http://cr.openjdk.java.net/~lmesnik/8203491/webrev.00/test/hotspot/jtreg/serviceability/sa/TestJmapCore.java.html

* Would you be adding JMapMetaspaceCore testing to this? Other than 
this, I think the jhsdb heapdump testing from vmTestbase has been covered.

* You might want to increase the timeout factor for this test. The test 
times out on my machine.

* You might want to consider removing the corefile and the heapdump 
files after the test execution (in the cases where the test passes).

* The following imports:
import jdk.test.lib.classloader.GeneratingClassLoader;
import java.util.Arrays;
import java.util.stream.Collectors;

are not needed.

* Nit:
// If any arguments are set prints pid so main process coud find corefile

coud --> could

* "Has not been able to find coredump. Test skipped."

One suggestion is to check if /proc/sys/kernel/core_pattern has a line 
starting with '|' and print a warning that a crash reporting tool might 
be used (Something like in ClhsdbCDSCore.java). But it is just a 
suggestion and you are free to ignore it. In due course, we could 
include this test also as a part of the consolidation of SA's corefile 
testing effort (https://bugs.openjdk.java.net/browse/JDK-8202297).


 > I have verified that test  still could reproduce issue
 > https://bugs.openjdk.java.net/browse/JDK-8176557 if default heap size is
 > large enough. However I think that it make a sense to add variant of
 > this test which explicit heap settings as a regression fix for bug
 > JDK-8176557 <https://bugs.openjdk.java.net/browse/JDK-8176557>.
 >

Sounds good to me.

 > Also I haven't managed to reproduce following issues with neither
 > existing or new tests:
 > https://bugs.openjdk.java.net/browse/JDK-8001227
 > https://bugs.openjdk.java.net/browse/JDK-8023376
 > https://bugs.openjdk.java.net/browse/JDK-8051445
 > Corresponding  tests were excluded for a long time so there are no any
 > results for them. I also see comments in bugs with unsuccessful attempts
 > to reproduce issues.  I think that it makes a sense just to remove these
 > entries from problemlist and file new open bugs if any of failures 
happen.

Sounds good to me.

Thanks,
Jini (Not a Reviewer).


On 5/23/2018 6:45 AM, Leonid Mesnik wrote:
> Hi
> 
> Could you please review following patch which rewrite 
> vmTestbase/heapdump tests as a jtreg java tests.
> 
> These heapdump tests were developed as a shell tests which verify 
> heapdump functionality in JDK 5. They work fine but needs some refactoring.
> 
> webrev: http://cr.openjdk.java.net/~lmesnik/8203491/webrev.00/
> bug: https://bugs.openjdk.java.net/browse/JDK-8203491
> 
> Below is high-level overview of existing tests and changes:
> 
> 1) Tests vmTestbase/heapdump/OnOOM* verify that VM correctly generates 
> heapdump when OOME in heap or metaspace happens. Also they verify that 
> -XX:HeapDumpPath= works correctly.
> Re-written as:
> http://cr.openjdk.java.net/~lmesnik/8203491/webrev.00/test/hotspot/jtreg/runtime/ErrorHandling/TestHeapDumpOnOutOfMemoryError.java.html
> Test provokes OOME and verifies generated heapdump.
> Please note that new jtreg test is very similar and use same test 
> patterns as most of ErrorHandling tests in 
> http://hg.openjdk.java.net/jdk/jdk/file/tip/test/hotspot/jtreg/runtime/ErrorHandling
> The main goal is to verify error handling when OOME is happen so I used 
> the same way to provoke OOME as most of other tests in this directory.
> New test might takes more then minute to complete test so I removed from 
> tier1.
> 
> 2) Tests vmTestbase/heapdump/JMap* verify that jmap correctly generates 
> heapdump from live process and core/mdmp file.
> There are existing tests which verify that jmap could correctly dump 
> heap for live process:
> http://hg.openjdk.java.net/jdk/jdk/file/f4735ff8d17d/test/hotspot/jtreg/serviceability/sa/TestHeapDumpForInvokeDynamic.java
> http://hg.openjdk.java.net/jdk/jdk/file/f4735ff8d17d/test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
> I believe that they provide the same coverage as heapdump/JMapHeap 
> and heapdump/JMapMetaspace tests and no new tests is needed.
> 
> The new test which verifies that jmap generate heapdump for corefile is:
> http://cr.openjdk.java.net/~lmesnik/8203491/webrev.00/test/hotspot/jtreg/serviceability/sa/TestJmapCore.java.html
> 
> Test provokes OOME to generate corefile (or mdml), generate heapdump 
> with jhsdb jmap and verify it. Previously tests send signals to provoke 
> coredump. Since JDK9 option 'CreateCoredumpOnCrash' could be used to 
> easily dump core on all platforms. Please note that test silently pass 
> if it doesn't find core file. Even if test tries to set ulimit -c 
> unlimited there is no guarantee that core file is generated and could be 
> found.
> I have verified that test  still could reproduce issue 
> https://bugs.openjdk.java.net/browse/JDK-8176557 if default heap size is 
> large enough. However I think that it make a sense to add variant of 
> this test which explicit heap settings as a regression fix for bug 
> JDK-8176557 <https://bugs.openjdk.java.net/browse/JDK-8176557>.
> 
> Also I haven't managed to reproduce following issues with neither 
> existing or new tests:
> https://bugs.openjdk.java.net/browse/JDK-8001227
> https://bugs.openjdk.java.net/browse/JDK-8023376
> https://bugs.openjdk.java.net/browse/JDK-8051445
> Corresponding  tests were excluded for a long time so there are no any 
> results for them. I also see comments in bugs with unsuccessful attempts 
> to reproduce issues.  I think that it makes a sense just to remove these 
> entries from problemlist and file new open bugs if any of failures happen.
> 
> I quickly looked through all resolved bugs  found by heapdump/ tests. 
> Most of them are related with different size limits overflow.  It might 
> be make a sense also create test which fill heap with large amount of 
> small objects to possible overflow of sizes of records. The similar 
> functionality was removed by JDK-8144137 
> <https://bugs.openjdk.java.net/browse/JDK-8144137>.  But I would prefer 
> to have separate RFE since it is not supported by existing tests.
> 
> Please let me know if it is possible to lost test coverage during this 
> conversion and more tests are needed. The more conservative way might be 
> to left existing tests for some time to see if if they still found 
> different bugs.
> 
> Testing:
> Run new tests on linux-x64,windows-x64,macos-x64.
> Run tier1/2.
> 
> 
> Leonid
> 
> 
> 
> 
>