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

[Mikhailo Seledtsov]

Hi Leonid,

   Thank you for review.

On 11/4/19, 8:27 PM, Leonid Mesnik wrote:
> 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/%7Emseledtsov/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/%7Emseledtsov/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' ?
Fixed.
>
> http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/gc/configuration/TestGCHeapConfigurationEventWithZeroBasedOops.java.udiff.html 
> <http://cr.openjdk.java.net/%7Emseledtsov/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/%7Emseledtsov/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/%7Emseledtsov/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;
Fixed
> Is run/othervm is really needed here:
> + * @run main/othervm jdk.jfr.event.os.TestInitialEnvironmentVariable
>
Fixed
>
> http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/jdk/jdk/jfr/event/runtime/TestVMInfoEvent.java.udiff.html 
> <http://cr.openjdk.java.net/%7Emseledtsov/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.
>
Agree. Using 'UseSerialGC' instead.
> http://cr.openjdk.java.net/~mseledtsov/8209813.00/test/lib/jdk/test/lib/util/JavaAgentBuilder.java.html 
> <http://cr.openjdk.java.net/%7Emseledtsov/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.
I will keep the order for now. However, I am planning to allow extra 
arguments after arg[0] and arg[1], which will be the additional 
attributes added to the manifest.
>
> 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.
     8233727: [TESTBUG] Review and update 
TestGCHeapConfigurationEventWithXYZ tests

I will make the changes, test and post an updated webrev shortly.

Thank you,
Misha
>
> Leonid
>
>> On Nov 4, 2019, at 7:48 PM, mikhailo.seledtsov at oracle.com 
>> <mailto:mikhailo.seledtsov at oracle.com> wrote:
>>
>> Ping
>>
>> On 10/25/19 5:00 PM, mikhailo.seledtsov at oracle.com 
>> <mailto: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 
>>> <http://cr.openjdk.java.net/%7Emseledtsov/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
>>>
>