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

[Ioi Lam]

On Fri, 13 Jan 2023 17:05:10 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> This is an enhancement of the test case in [JDK-8296754](https://bugs.openjdk.org/browse/JDK-8296754), which tests against an archive created by the "boot JDK", which is usually set as the previous official JDK release when building the JDK repo.
>> 
>> If it's able to acquire previous valid JDK releases:
>>      - Download and install previous JDK versions (19 through N)
>>         where N == java.lang.Runtime.version​().major() - 1
>>      - Test the interaction of the current JDK versus each of the previous releases
>> 
>> If it's not able to find the previous releases revert to the existing logic in TestAutoCreateSharedArchiveUpgrade.java (use the test.boot.jdk or test.previous.jdk properties). Verified with tier1-4 tests.
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added exceptions instead of default jdk

Changes requested by iklam (Reviewer).

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java line 104:

> 102:         newJVM = TEST_JDK + FS + "bin" + FS + "java";
> 103: 
> 104:         // Example path: bundles/linux-x64/jdk-19_linux-x64_bin.tar.gz/jdk-19/bin/java

It's not cleat what this comment refers to. I think it can be deleted.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java line 172:

> 170: 
> 171:     // Fetch JDK artifact depending on platform
> 172:     // If the artifact cannot be found, default to the test.boot.jdk property

This comment about test.boot.jdk is no longer valid. If the artifact cannot be found, RuntimeException is thrown.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java line 199:

> 197:             architecture = "x";
> 198:         } else if (Platform.isAArch64()) {
> 199:             architecture = "aarch";

It's better to use "x64" and "aarch64" here. The comments below should be fixed, too.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java line 207:

> 205:         // Ex: bundles/linux-x64/jdk-19_linux-x64_bin.tar.gz
> 206:         if (Platform.isWindows()) {
> 207:             jdkArtifactMap.put("file", "bundles/windows-x64/jdk-" + version + "_windows-x64_bin.zip");

Instead of hard-coding "x64", `architecture` should be used.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java line 225:

> 223:         try {
> 224:             path = ArtifactResolver.resolve("jdk", jdkArtifactMap, true) + "/jdk-" + version;
> 225:             System.out.println("Boot JDK path: " + path);

`String path` should be declared inside this `try` block and its value should be returned here. That way you don't need to read all the catch block below to find out what value is actually returned.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java line 230:

> 228:             if (cause == null) {
> 229:                 System.out.println("Cannot resolve artifact, "
> 230:                         + "please check if JIB jar is present in classpath.");

If we come to here, the function would end up returning null. We shold always throw the RuntimeException  when we come into this `catch` block.

Also, I am not sure if we should assume that `e.getCause() == null` has any specific meaning. There could be multiple implementations of `ArtifactResolver.resolve()` and we can't predict what the `e.getCause()` would be.

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

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