RFR: 8156762: compiler/ciReplay/TestVM_no_comp_level.java fails with - 'Unexpected exit code for negative case: [-client]: expected 0 to not equal 0'

[Rahul Raghavan]

Hi,

Thank you Zoltan for review comments.
Yes made the required change.

<Updated webrev.01> - http://cr.openjdk.java.net/~rraghavan/8156762/webrev.01/

Thanks,
Rahul

> -----Original Message-----
> From: Zoltán Majó
> Sent: Thursday, December 08, 2016 1:55 PM
> To: Rahul Raghavan; hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR: 8156762: compiler/ciReplay/TestVM_no_comp_level.java fails with - 'Unexpected exit code for negative case: [-
> client]: expected 0 to not equal 0'
> 
> Hi Rahul,
> 
> 
> can you please move the else on line 71 to line 70 (to the same line
> with the closing '}')?
> 
>    68             if (SERVER_VM_AVAILABLE) {
>    69                 negativeTest(CLIENT_VM_OPTION);
>    70             }
>    71             else {
>    72                 positiveTest(CLIENT_VM_OPTION);
>    73             }
> 
> Otherwise it looks good to me.
> 
> Thank you!
> 
> Best regards,
> 
> 
> Zoltan
> 
> On 12/08/2016 09:10 AM, Rahul Raghavan wrote:
> > Hi,
> >
> > Please review following patch for JDK-8156762.
> > <webrev.00> -  http://cr.openjdk.java.net/~rraghavan/8156762/webrev.00/
> >
> > <jbs> - https://bugs.openjdk.java.net/browse/JDK-8156762
> >
> > Please note that JDK-8156762 is an Oracle closed bug,
> >    reporting failure for hotspot/test/compiler/ciReplay/TestVM_no_comp_level.sh
> >    for build configuration with only Client VM.
> >
> > Understood the test scenario as
> >      - force JVM to generate test_replay.txt file
> >      - remove compilation level from test_replay.txt file
> >      - then check replay the modified test_replay.txt
> >
> > Above proposed test fix in webrev.00 -
> >      [test/compiler/ciReplay/TestVMNoCompLevel.java]
> >          ..............
> >          public void testAction() {
> >              ......
> >              ......
> >              if (CLIENT_VM_AVAILABLE) {
> >      -           negativeTest(CLIENT_VM_OPTION);
> >      +           if (SERVER_VM_AVAILABLE) {
> >      +               negativeTest(CLIENT_VM_OPTION);
> >      +           }
> >      +           else {
> >      +               positiveTest(CLIENT_VM_OPTION);
> >      +           }
> >              }
> >              if (SERVER_VM_AVAILABLE) {
> >                  positiveTest(TIERED_DISABLED_VM_OPTION, SERVER_VM_OPTION);
> >                  positiveTest(TIERED_ENABLED_VM_OPTION, SERVER_VM_OPTION);
> >              }
> >          }
> >          ..............
> >
> > The test was written expecting VM to crash always in Client VM available case,
> >    when replay the modified test_replay.txt without compilation level.
> > But found this is not true because a default compiler level of highest available level value is always used, if the same is missing,
> >    to support older version of Compiler replay without compiler level support.
> > [process_compile() - hotspot\src\share\vm\ci\ciReplay.cpp]
> >
> > So for the reported case if ONLY CLIENT_VM_AVAILABLE is true -
> > Originally in test, VM was expected to crash always here.
> > Found this is wrong due to the setting of default compiler level
> >    to highest available level value in process_compile() - which is 'CompLevel_simple' in this case.
> > So for ONLY CLIENT_VM_AVAILABLE case the VM will not crash at 'is_valid_comp_level()',
> >    going ahead with replaying the modified test_replay.txt with -client option.
> > (.i.e.   positiveTest(-client) is required)
> >
> > For the case with if both CLIENT_VM_AVAILABLE and SERVER_VM_AVAILABLE are true,
> > when test_replay.txt without compilation level is used,
> > the default compiler level assigned in process_compile() will be 'CompLevel_full_optimization'.
> > So when attempted replay the modified test_replay.txt with -client option,
> >    the VM should fail  for 'if (CLIENT_VM_AVAILABLE)' block at -
> >       if (!TieredCompilation && (comp_level != CompLevel_highest_tier)) check [is_valid_comp_level()]
> > (.i.e.   negativeTest(-client) is required)
> >
> > The further testing for SERVER_VM_AVAILABLE is not touched
> > and any how will always continue with required tiered disable/enabled , -server options.
> >
> > No issues with testing for various targets.
> >
> > Thanks,
> > Rahul
>