RFR(S): 8209813: [TESTBUG] rewrite JFR shell tests in Java

[Leonid Mesnik]

Hi

Sorry for late answer. See my comments:

http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/gc/configuration/TestGCHeapConfigurationEventWith32BitOops.java.udiff.html <http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/gc/configuration/TestGCHeapConfigurationEventWith32BitOops.java.udiff.html>

No comments.

http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/gc/configuration/TestGCHeapConfigurationEventWithHeapBasedOops.java.udiff.html <http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/gc/configuration/TestGCHeapConfigurationEventWithHeapBasedOops.java.udiff.html>

Line
+ * @run main/othervm -XX:+UnlockExperimentalVMOptions -XX:-UseFastUnorderedTimeStamps -XX:+UseParallelGC -XX:+UseParallelOldGC -XX:+UseCompressedOops -Xmx100m -Xms100m -XX:InitialHeapSize=100m -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI jdk.jfr.event.gc.configuration.TestGCHeapConfigurationEventWith32BitOops

confuses me. Do you want to run 'TestGCHeapConfigurationEventWithHeapBasedOops' ? 

http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/gc/configuration/TestGCHeapConfigurationEventWithZeroBasedOops.java.udiff.html <http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/gc/configuration/TestGCHeapConfigurationEventWithZeroBasedOops.java.udiff.html>

No comments

http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/io/EvilInstrument.java.udiff.html <http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/io/EvilInstrument.java.udiff.html>

No comments

http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/os/TestInitialEnvironmentVariable.java.udiff.html <http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/os/TestInitialEnvironmentVariable.java.udiff.html>

The order is incorrect
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
Is run/othervm is really needed here:
+ * @run main/othervm jdk.jfr.event.os.TestInitialEnvironmentVariable


http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/runtime/TestVMInfoEvent.java.udiff.html <http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/runtime/TestVMInfoEvent.java.udiff.html>

+    public static void generateFlagsFile() throws Exception {
+        Files.writeString(Paths.get("", "TestVMInfoEvent.flags"), "-UseCompressedOops");
+    }


The coops are not supported on 32-bit platforms, so you need to add additonal  checks or use other flag which supported anywhere.

http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/lib/jdk/test/lib/util/JavaAgentBuilder.java.html <http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/lib/jdk/test/lib/util/JavaAgentBuilder.java.html>

I think it would be better to change class <-> jar names. So you could extend arguments to set jar, premain/agent class and other classes if needed.  however it is just my opinion. 

Also it is good time to file RFE to review these tests later. It is not clear is it worth to have the ParallelGC only is tested and why macOS is not supported.

Leonid

> On Nov 4, 2019, at 7:48 PM, mikhailo.seledtsov at oracle.com wrote:
> 
> Ping
> 
> On 10/25/19 5:00 PM, mikhailo.seledtsov at oracle.com wrote:
>> Please review this change that converts JFR shell tests to Java.
>> 
>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8209813
>>     Webrev: http://cr.openjdk.java.net/~mseledtsov/8209813.00/index.html
>>     Testing:
>>         1. Locally: exercised affected tests locally on Linux-x64: PASS
>>         2. Test Cluster: jdk/jdk/jfr test run - in progress
>> 
>> 
>> Thank you,
>> Misha
>>