RFR: 8373661: Add edge case tests for Objects.requireNonNull methods [v4]

eunbin son duke at openjdk.org
Sat Dec 20 06:13:04 UTC 2025


On Wed, 17 Dec 2025 22:43:44 GMT, Chen Liang <liach at openjdk.org> wrote:

>> eunbin son has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8373661: Use assertSame for exception instance verification
>>   
>>   Updated testRequireNonNullWithSupplierThrowingException to allocate
>>   the exception outside of the lambda and use assertSame to verify
>>   the same exception instance is thrown, as suggested by @liach.
>>   
>>   This provides a more precise verification that the exception from
>>   the supplier is thrown directly, not wrapped.
>>   
>>   Thanks to @liach for the feedback.
>
> I agree with Roger's points about simplifying some test messages. Sometimes we don't even write test messages if the subject is clear from the test code context, because the exceptions usually include the line numbers already. However, these are not critical.
> 
> However, I think Roger might be too strict on imports and Assertions.fail vs exceptions - these are acceptable because they don't affect the test quality, so I am not concerned.

Thank you for the feedback, @liach and @RogerRiggs! I've addressed your concerns:

- [x] Updated description contents and formatting , copyright year (as requested by @RogerRiggs)
- [x] Removed exception message tests to allow API flexibility (as suggested by @liach)
- [x] Removed verbose Javadoc and inline comments (as suggested by @liach)
- [x] Simplified test messages to use method names only 

The tests now follow OpenJDK patterns where only exception types are tested, not messages. The code is more concise and maintainable.

The changes have been pushed. Please review again.

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

PR Comment: https://git.openjdk.org/jdk/pull/28845#issuecomment-3677444777


More information about the core-libs-dev mailing list