[jdk8u-dev] RFR: 8208655: use JTreg skipped status in hotspot tests [v2]

[Thomas Stuefe]

On Fri, 24 Nov 2023 05:21:33 GMT, Andrew John Hughes <andrew at openjdk.org> wrote:

>> This backport brings in `jtreg.SkippedException` which is used by JDK-8308592 (and likely other potential future backports as well). It is a test-only change which allows a skipped test to be correctly reported to jtreg, rather than it appearing like a successful run.
>> 
>> The actual code changes to the HotSpot tests to use `SkippedException` were mostly clean. The main manual adaptations involved the placement of `import` statements (8u also doesn't have 8132919: 'Put compiler tests in packages') and copyright headers. Most tests also needed to have the library location `/test/lib` added. This is because the HotSpot tests are currently still using the HotSpot test library, not the top-level library. @zzambers work on this should eventually remove the need to reference both libraries. 
>> 
>> I've included with this change a number of follow-on changes which were necessary to make the changed tests pass. I don't know how the original version of 8208655 was committed as is, because it contains a number of typos and missing import statements which cause tests to fail.  The follow-up changes are:
>> 
>> * 8023735 backport. This change introduces the code which is altered by 8208655 to use `SkippedException`. The fix is simple enough to just backport with this change and fix the test in the process. It applied cleanly other than the `@ignore` line which has been replaced in 8u by a `ProblemList.txt` addition which is removed by this backport instead.
>> * Addition of `vm.debug`; this is cherry-picked from 8155219: "[TESTBUG] Rewrite compiler/ciReplay/TestVM.sh in java" which first introduced it in later JDKs. The implementation needs to be adapted to work around the absence of `jdk.debug` (JDK-8139986). Using `java.vm.version` matches what happens in the `Platform.java` test library code.
>> * 8208701: first fix for typos in 8208655 as committed. Some changes had to be manually applied due to context differences but otherwise a clean backport.
>> * 8208706: fixes a missing `import` line in `ConstantGettersTransitionsTest.java`, which was applied manually due to differing context.
>> * 8213410 backport which is necessary as 8208655 removes a number of checks altogether, replacing them with `@requires` statements. This is why we need `vm.debug` above and also this change due to the removed 32-bit check in `runtime/CompressedOops/CompressedClassPointers.java`
>> * For consistency, the debug build check in `TestLargePageUseForAuxMemory.java` was replaced with vm.debug. I...
>
> Andrew John Hughes has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - 8183503: Update hotspot tests to allow for unique test classes directory
>  - 8186095: upgrade to jtreg 4.2 b08
>  - 8129355: [TESTBUG] runtime FragmentMetaspaceSimple.java fails with java.lang.ClassNotFoundException: test.Empty

Looked through the tests, mainly with an eye for required/skipped replacements of existing manual test exclusion. Also looked a the libjsig test a bit more closely. I did not directly compare all patches against the new one (now I understand why you maintainers dislike compound patches so much).

hotspot/test/gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java line 28:

> 26:  * @bug 8004924
> 27:  * @summary Checks that jmap -heap contains the flag CompressedClassSpaceSize
> 28:  * @requires vm.bits == 64

Not sure about this. The flag itself exists also on 32-bit, so the test should work there; it is pointless though since the flag is never used.

hotspot/test/gc/metaspace/TestMetaspaceMemoryPool.java line 33:

> 31:  * @summary Tests that a MemoryPoolMXBeans is created for metaspace and that a
> 32:  *          MemoryManagerMXBean is created.
> 33:  * @requires vm.bits == 64

Why? We have metaspace also on 32-bit, just no class space.

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

PR Review: https://git.openjdk.org/jdk8u-dev/pull/387#pullrequestreview-1747447134
PR Review Comment: https://git.openjdk.org/jdk8u-dev/pull/387#discussion_r1404033873
PR Review Comment: https://git.openjdk.org/jdk8u-dev/pull/387#discussion_r1404032827