RFR(S) 8193664 [10] - AppCDS tests should use -XX:+UnlockCommercialFeatures when running with commercial JDK

[Ioi Lam]

On 1/10/18 10:04 PM, David Holmes wrote:
> Hi Ioi,
>
> On 11/01/2018 2:42 PM, Ioi Lam wrote:
>> Hi David,
>>
>> Thanks for the review. I've removed the duplicated code and added a 
>> new method TestCommon.makeCommandLineForAppCDS. The code looks like 
>> this now:
>>
>> !  ProcessBuilder dumpPb = ProcessTools.createJavaProcessBuilder(true,
>> !    TestCommon.makeCommandLineForAppCDS(
>>         "-XX:+UseAppCDS",
>>         "-XX:+UseCompressedOops",
>>         "-XX:+UseG1GC",
>>         "-cp", appJar,
>>         "-XX:SharedArchiveConfigFile=" + sharedArchiveConfigFile,
>>         "-XX:SharedArchiveFile=./SharedStringsBasic.jsa",
>>         "-Xshare:dump",
>> !      "-Xlog:cds,cds+hashtables"));
>
> Looks good! Thanks for that.
>
> One comment:
>
>  public static String[] makeCommandLineForAppCDS(String... args) 
> throws Exception {
>
> why the throws clause?
>
Oops, that was a mistake. I'll remove it.

Thanks
- Ioi
> Thanks,
> David
>
>> Updated webrev is at
>>
>> http://cr.openjdk.java.net/~iklam/jdk10/8193664-commercial-appcds-testbug.v02/ 
>>
>>
>> I also updated the copyright years as mentioned by Jiangli.
>>
>> Thanks
>> - Ioi
>>
>>
>> On 1/10/18 2:31 PM, David Holmes wrote:
>>> Hi Ioi,
>>>
>>> I have two small reservations ...
>>>
>>> 1. The duplicate definitions of commercial()
>>> 2. The use of -showversion in the openJDK case. Might not this 
>>> interfere with output parsing?
>>>
>>> Thanks,
>>> David
>>>
>>> On 11/01/2018 5:47 AM, Ioi Lam wrote:
>>>> Hi,
>>>>
>>>> Please review this small change in the AppCDS tests
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8193664
>>>> http://cr.openjdk.java.net/~iklam/jdk10/8193664-commercial-appcds-testbug.v01/ 
>>>>
>>>>
>>>> Summary:
>>>>
>>>>      * This patch changes tests ONLY. There's no code change.
>>>>      * The change affects ONLY the commercial JDK built by Oracle.
>>>>      * It does not affect the OpenJDK
>>>>
>>>>      * Oracle commercial JDK 10 still requires 
>>>> -XX:+UnlockCommercialFeatures
>>>>        to be specified when using AppCDS tests. This requirement 
>>>> will be
>>>>        removed soon in the future, but meanwhile, we need to add a few
>>>>        -XX:+UnlockCommercialFeatures flags in the test cases to 
>>>> make them pass.
>>>>
>>>> Tested under both Oracle commercial JDK and OpenJDK builds. All 
>>>> affected
>>>> tests passed in both configurations.
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>>
>>