Hi Leonid,

Looks good in general.


http://cr.openjdk.java.net/~lmesnik/8230942/webrev.01/test/lib/jdk/test/lib/SA/SATestUtils.java.udiff.html

Minor:
+import java.io.File;
 import java.io.IOException;
-import java.util.List;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
Newly added "import java.io.File;" is out of order.

The order of imports in the other two files is also not always correct.

For instance, the imports for Platform, SA.SATestUtils and Utils below are out of order:
 import jdk.test.lib.Platform;
 import jdk.test.lib.classloader.GeneratingClassLoader;
 import jdk.test.lib.hprof.HprofParser;
 import jdk.test.lib.process.ProcessTools;
 import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.SA.SATestUtils;
 import jdk.test.lib.Utils;
 import jtreg.SkippedException;

No need in new webrev if you fix it.

Thanks,
Serguei



On 9/26/19 23:28, Leonid Mesnik wrote:
Thanks for feedback.

I checked time execution. Tests takes several seconds longer on the hosts which compress cores. Also, tests TestJmapCore.java, TestJmapCoreMetaspace.java are slow and not included in tier1_serviceability anyway.

Updated version here. I fixed it accordingly with your comments:

Leonid

On Sep 26, 2019, at 7:00 PM, David Holmes <[email protected]> wrote:

Hi Leonid,

On 27/09/2019 7:18 am, Leonid Mesnik wrote:
Hi
Some hosts used for JDK testing have customized core dump settings. They compress core files saved in current directory on-the-fly to reduce required disk space.
This fix adopt several SA tests, trying to unpack core.pid.gz before test process it with jhsdb. It affects only execution in the case if core.pid.gz files are actually generated.
Verified that tests are passed and not skipped anymore on default and new configurations.
webrev: http://cr.openjdk.java.net/~lmesnik/8230942/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8230942

Overall seems fine. I hope it doesn't take too long to do the unzipping. :)

A few minor items

test/lib/jdk/test/lib/SA/SATestUtils.java

+         for(File gzCore : gzCores) {

Nit: add space after for

+             } catch (IOException e) {
+                 throw new SkippedException("Not able to unzip core file.");
+             }

Please add the IOException as a cause for the SkippedException so that we have some diagnostics on why it couldn't be unzipped.

---

test/hotspot/jtreg/serviceability/sa/TestJmapCore.java

32 import java.io.File;

File is already imported at line 46.

+         SATestUtils.unzipCores(new File("."));
...
         File[] cores = new File(".").listFiles((dir, name) -> name.matches(pattern));

Suggest:

File pwd = new File(".");
SATestUtils.unzipCores(pwd);
...
File[] cores = pwd.listFiles((dir, name) -> name.matches(pattern));

and also at line 117:
                    + ": " + String.join(",", pwd.list()) + ".");

Thanks,
David
-----

Leonid



Reply via email to