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

[Leonid Mesnik]

Looks good.

Leonid

On 11/6/19 11:06 AM, Mikhailo Seledtsov wrote:
> 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
>>>>
>>