RFR: 8346929: runtime/ClassUnload/DictionaryDependsTest.java fails with "Test failed: should be unloaded" [v2]

[David Holmes]

On Wed, 8 Jan 2025 20:50:53 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Adopt the new ClassUnloadCommon.triggerUnloading API to make the unloading tests more reliable.
>> Tested with tier1 on all platforms.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed @calvincheung comments.

Generally looks good but there are a few inconsistencies with the changes.

Thanks

test/hotspot/jtreg/runtime/ClassUnload/KeepAliveClass.java line 75:

> 73:     ClassUnloadCommon.triggerUnloading();
> 74: 
> 75:     Set<String> aliveClasses = ClassUnloadCommon.triggerUnloading(List.of(className));

The call to `triggerUnloading` at line 73 is redundant because you don't check the result afterwards and the call here will repeat it.

test/hotspot/jtreg/runtime/ClassUnload/KeepAliveSoftReference.java line 40:

> 38: import jdk.test.lib.classloader.ClassUnloadCommon;
> 39: import java.util.List;
> 40: import java.util.Set;

This is unused in this test, but it may be better to check the returned set anyway rather than calling `wb.isClassAlive` at line 76.

test/hotspot/jtreg/runtime/ClassUnload/SuperDependsTest.java line 80:

> 78:         SuperDependsTest d = new SuperDependsTest();
> 79:         d.test();
> 80:         System.out.println("Should unload MyTest and p2.c2 just now");

With the removal of line 78 the println is no longer relevant/correct/useful.

test/hotspot/jtreg/runtime/ClassUnload/UnloadInterfaceTest.java line 93:

> 91: 
> 92:         // Now unload className. This calls triggerUnloading but we only pass the class we expect to be unloaded
> 93:         // otherwise the test will take too long.

Don't understand why this one needs a comment and none of the others do. ?? Also why the "otherwise" part?

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22974#pullrequestreview-2538665342
PR Review Comment: https://git.openjdk.org/jdk/pull/22974#discussion_r1908113633
PR Review Comment: https://git.openjdk.org/jdk/pull/22974#discussion_r1908114997
PR Review Comment: https://git.openjdk.org/jdk/pull/22974#discussion_r1908115774
PR Review Comment: https://git.openjdk.org/jdk/pull/22974#discussion_r1908116664