RFR(S): 8243947: [TESTBUG] hotspot/jtreg:hotspot_appcds_dynamic fails when the JDK doesn't have default CDS archive

[Ioi Lam]

Hi Calvin,

This looks good to me.

Thanks
- Ioi

On 5/13/20 5:51 PM, Calvin Cheung wrote:
> Hi Ioi,
>
> Thanks for your review.
>
> Here's an update webrev based on your comments:
>
>     http://cr.openjdk.java.net/~ccheung/jdk15/8243947/webrev.01/
>
> I've modified 2 files: TestCommon.java and DynamicArchiveTestBase.java.
>
> thanks,
>
> Calvin
>
> On 5/13/20 2:52 PM, Ioi Lam wrote:
>> Hi Calvin,
>>
>> I think this code can be changed to make it easier to understand:
>>
>>
>>  205             if (tempBaseArchive != null) {
>>  206                 baseArchive = new File(tempBaseArchive);
>>  207             }
>>  208             if (tempBaseArchive == null || !baseArchive.isFile()) {
>>  209                 tempBaseArchive = 
>> getNewArchiveName("tempBaseArchive");
>>
>> ->
>>
>> if (tempBaseArchive == null || !(new File(tempBaseArchive)).isFile()) {
>>   tempBaseArchive = getNewArchiveName("tempBaseArchive");
>>
>>
>> Is the following necessary? I think it should already be handled in 
>> dump2. (Similarly for the "run" and "dumpAndRun" methods as well).
>>
>>  136     public static Result dump(String topArchiveName, String ... 
>> cmdLineSuffix)
>>  137         throws Exception
>>  138     {
>>  139         if (!isUseSharedSpacesDisabled()) {
>>  140             return dump2(null, topArchiveName, cmdLineSuffix);
>>  141         } else {
>>  142             String baseArchiveName = getTempBaseArchive();
>>  143             return dump2(baseArchiveName, topArchiveName, 
>> cmdLineSuffix);
>>  144         }
>>
>> The rest of the changes look good to me.
>>
>> Thanks
>> - Ioi
>>
>>
>> On 5/7/20 12:17 PM, Calvin Cheung wrote:
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8243947
>>> webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8243947/webrev.00/
>>>
>>> This change is to handle the situation when a JDK doesn't have 
>>> default CDS archive or the CDS archive was created with a different 
>>> vm options than the ones supplied for running the test. This change 
>>> also address JDK-8243207: Increase AppCDS test coverage with ZGC.
>>>
>>> Summary of changes:
>>>
>>> - In DynamicArchiveTestBase.java, it checks if the UseSharedSpaces 
>>> flag has been disabled using WhiteBox api. If so, it will regenerate 
>>> a base CDS archive before running the dynamic CDS archive test.
>>> - Since DynamicArchiveTestBase uses WhiteBox, all tests under 
>>> dynamicArchive will need to be run with WhiteBox. Changes to the 
>>> tests under dynamicArchive are mostly adding WhiteBox to the command 
>>> line.
>>> - In TestCommon.java, for AppCDS tests running with the 
>>> -Dtest.dynamic.cds.archive=true option, a base CDS archive will be 
>>> generated.
>>>
>>> Testing:
>>>
>>>     locally on linux-x64, ran appcds test group 
>>> hotspot_appcds_dynamic with 
>>> vmoptions="-Dtest.dynamic.cds.archive=true -XX:+UseZGC"
>>>            and with the default CDS archive (classes.jsa) removed 
>>> from the test JDK
>>>     mach5 tier1 - 4.
>>>
>>> thanks,
>>>
>>> Calvin
>>>
>>>
>>