RFR: 8287873: Add test for using -XX:+AutoCreateSharedArchive with different JDK versions [v2]

[Ioi Lam]

On Fri, 6 Jan 2023 13:47:22 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Matias Saavedra Silva has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
>> 
>>  - Fixed jib.sh
>>  - Removed prints
>>  - Ioi comments and jib.sh restored
>>  - Merge branch 'master' into sharedArchiveTest_8287873
>>  - Removing unused code
>>  - Removed file added by mistake
>>  - Defaults to old functionality on failure
>>  - 8287873: Add test for using -XX:+AutoCreateSharedArchive with different JDK versions
>
> test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java line 62:
> 
>> 60:     // "make test TEST=test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java",
>> 61:     // the test.boot.jdk property is normally passed by make/RunTests.gmk
>> 62:     private static String BOOT_JDK;
> 
> Could we please call this something else? The concept of the BOOT_JDK is a rather specific thing in the JDK build process. This test is using the build's BOOT_JDK as a default for "a JDK of some older version than the current". Calling that "BOOT_JDK" in this test is confusing at least to me.
> 
> Perhaps something like `OLD_JDK` would be more suitable?

Actually OLD_JDK would be confusing because we also have PREV_JDK. This is the original code before this PR:


// If you're running this test manually, specify the location of a previous version of
// the JDK using "jtreg -vmoption:-Dtest.previous.jdk=${JDK19_HOME} ..."
private static final String PREV_JDK = System.getProperty("test.previous.jdk", null);

// If you're unning this test using something like
// "make test TEST=test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java",
// the test.boot.jdk property is passed by make/RunTests.gmk
private static final String BOOT_JDK = System.getProperty("test.boot.jdk", null);


So the problem with this PR is it overloads the meaning of BOOT_JDK. I think it's better to do something like this:


static void setupJVMs(int fetchVersion) throws Throwable {
    ....
    if (fetchVersion> 0) {
        oldJVM = fetchJDK(fetchVersion) + FS + "bin" + FS + "java";
    } else if (PREV_JDK != null) {
        oldJVM = PREV_JDK + FS + "bin" + FS + "java";
    } else if (BOOT_JDK != null) {
        oldJVM = BOOT_JDK + FS + "bin" + FS + "java";
    } else {

-------------

PR: https://git.openjdk.org/jdk/pull/11852