On Mon, 5 Jan 2026 14:06:50 GMT, Jorn Vernee <[email protected]> wrote:

>> Chen Liang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Some omissions
>
> test/jdk/java/lang/invoke/LoopCombinatorTest.java line 236:
> 
>> 234:             assertEqualsFIXME(messageOrNull, iae.getMessage());
>> 235:             return;
>> 236:         }
> 
> Could you split this into separate positive and negative tests, with separate 
> sources?

Done.

> test/jdk/java/lang/invoke/MethodHandles/TestDropReturn.java line 40:
> 
>> 38: import org.junit.jupiter.params.provider.MethodSource;
>> 39: 
>> 40: @TestInstance(TestInstance.Lifecycle.PER_CLASS)
> 
> Why is this annotation needed here? There's only one test method.

Reviewed `@TestInstance`, now only exists on VarHandle tests which are harder 
to restructure.

> test/jdk/java/lang/invoke/condy/CondyBSMInvocation.java line 241:
> 
>> 239:             var e = Assertions.assertThrows(BootstrapMethodError.class, 
>> mh::invoke);
>> 240:             Throwable t = e.getCause();
>> 241:             
>> Assertions.assertTrue(WrongMethodTypeException.class.isAssignableFrom(t.getClass()));
> 
> Suggestion:
> 
>             assertInstanceOf(WrongMethodTypeException.class, e.getCause());

Migrated some possible sites to use assertInstanceOf.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28879#discussion_r2710458166
PR Review Comment: https://git.openjdk.org/jdk/pull/28879#discussion_r2710457928
PR Review Comment: https://git.openjdk.org/jdk/pull/28879#discussion_r2710456940

Reply via email to