[lworld] RFR: 8363846: [lworld] Make Class.isIdentityClass() non-native [v3]

Coleen Phillimore coleenp at openjdk.org
Wed Jul 23 16:51:22 UTC 2025


On Wed, 23 Jul 2025 16:04:18 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix test to use junit and test without preview mode.
>
> test/jdk/valhalla/valuetypes/IsIdentityClassTest.java line 52:
> 
>> 50: 
>> 51:     private static void assertFalseIfPreview(boolean condition, String msg) {
>> 52:         if (PreviewFeatures.isEnabled()) {
> 
> We can use `assertEquals(!PreviewFeatures.isEnabled(), msg)`

Thats a good suggestion.

> test/jdk/valhalla/valuetypes/IsIdentityClassTest.java line 63:
> 
>> 61:         RuntimeMXBean runtimeMxBean = ManagementFactory.getRuntimeMXBean();
>> 62:         List<String> arguments = runtimeMxBean.getInputArguments();
>> 63:         boolean UseArrayFlattening = !arguments.contains("-XX:-UseArrayFlattening");
> 
> Optional comments:
> 
> I wonder if it's more usually to obtain flags with serviceability, or explicitly pass -D properties...
> 
> Also for these global states, it's better to capture them with a `@Setup static void setup()` method and store in static fields, especially if multiple tests use these states.

I copied this out of a hotspot valhalla test.  If I remove the flattening tests, I suppose I won't need this anymore.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1514#discussion_r2226134801
PR Review Comment: https://git.openjdk.org/valhalla/pull/1514#discussion_r2226137286


More information about the valhalla-dev mailing list