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
