RFR(M): 8197959: [TESTBUG] Some (App)CDS tests requires modification due to there removal of the Java EE and CORBA modules

[Calvin Cheung]

Hi Ioi,

Thanks for your review.

On 4/19/18, 10:29 PM, Ioi Lam wrote:
> Hi Calvin,
>
> the changes look good. I have a few comments:
>
> [1] I think "@modules java.compiler" be added to all tests that use 
> javax/annotation, which is provided by the java.compiler module.
I've added the missing "@modules java.compiler".
>
>
> [2] CheckAnonymousClass.java:
>
> I believe the old behavior was that when "--add-modules java.corba" 
> was specified during dump time, some Lambda expressions were executed 
> and thus some VM anonymous classes were loaded as a side effect.
>
> With the new version, I don't think any Lambda expressions are 
> executed during dump time (because the normal module bootstrap code 
> has been very careful in avoiding Lambda). Therefore, the new version 
> doesn't test the intended behavior.
>
> The AnonVmClassesDuringDump.java test forces Lambda expressions to be 
> executed during dump time (by using Java agents). Maybe the pattern 
> matching in  CheckAnonymousClass.java can be moved inside 
> AnonVmClassesDuringDump.java? Then, we can delete 
> CheckAnonymousClass.java
I've moved the pattern matching logic from CheckAnonymousClass.java to 
AnonVmClassesDuringDump.java and delete the CheckAnonymousClass.java.
>
> [3] For this code:
>
> +        CDSTestUtils.Result res = TestCommon.runWithModules(
> +                                      prefix,
> + UPGRADEDMODS_DIR[upgradeModIdx].toString(),
> +                                      MODS_DIR.toString(),
> +                                      mid,
> +                                      archiveClass, loaderName, 
> "true"); // last 3 args passed to test
> +
> +        output = res.getOutput();
> +        try {
> +            output.shouldContain(expectedException);
> +        } catch (Exception e) {
> +            TestCommon.checkCommonExecExceptions(output, e);
>          }
>
>    Is it because you don't care about the exit code? If so, you can do 
> this:
>
>
>         TestCommon.runWithModules(...)
>           .ifNoMappingFailure(out -> 
> out.shouldContain(expectedException));
>
>
>     Then you don't need to directly access the output.
I've also made similar changes to another usage of 
TestCommon.runWithModules() at lines 203-209 so that the 
CDSTestUtils.getOuput method is no longer needed.

Updated webrevs:

     incremental: http://cr.openjdk.java.net/~ccheung/8197959/webrev.00_01/

     full: http://cr.openjdk.java.net/~ccheung/8197959/webrev.01/

thanks,
Calvin
>
> Thanks
> - Ioi
>
>
> On 4/18/18 3:32 PM, Calvin Cheung wrote:
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8197959
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/8197959/webrev.00/
>>
>> Fixing the affected CDS and AppCDS tests due to the removal of the 
>> Java EE and CORBA modules.
>> Also removing the unneeded helper classes.
>>
>> It passed hs-tier1 through tier3 tests.
>>
>> thanks,
>> Calvin
>