On Thu, 18 Dec 2025 11:54:41 GMT, eunbin son <[email protected]> wrote:

>> ## Summary
>> Adds comprehensive edge case tests for `Objects.requireNonNull`, 
>> `requireNonNullElse`, and `requireNonNullElseGet` methods to improve 
>> test coverage.
>> 
>> ## Problem
>> The current test suite for `Objects.requireNonNull` methods covers 
>> basic cases but lacks edge case coverage.
>> 
>> ## Solution
>> This PR adds tests for the following edge cases:
>> - requireNonNull with null Supplier parameter
>> - requireNonNull with Supplier that throws exception
>> - requireNonNullElse with both arguments null
>> - requireNonNullElseGet with null supplier
>> - requireNonNullElseGet with supplier returning null
>> 
>> ## Issue
>> Fixes JDK-8373661
>> 
>> **JBS Issue Link**: 
>> https://bugs.java.com/bugdatabase/view_bug?bug_id=JDK-8373661
>> 
>> ## Type of Change
>> - [x] Test addition/modification
>> - [ ] Bug fix
>> - [ ] New feature
>> - [ ] Documentation improvement
>> - [ ] Refactoring
>> 
>> ## Testing
>> 
>> make test TEST="jtreg:test/jdk/java/util/Objects"
>
> eunbin son has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8373661: Address code style feedback
>   
>   - Expanded wildcard imports to individual imports
>   - Simplified test messages to identify tests only
>   - Used Assertions.fail() where appropriate
>   - Kept assertTrue/assertFalse for isNull API tests
>   
>   Thanks to @RogerRiggs and @liach for the feedback.

I don't think we can accept this patch as-is - Even though tests would 
"increase coverage", this patch adds a lot of messages that are probably never 
printed that we cannot really validate them.

In particular, this patch is verbose - a lot of the message strings and 
comments are simply repeating what is immediately clear from the code. Such 
spam of information reminds people of large language models, which tend to 
generate code that seems rich on the first glance but ultimately becomes 
problematic in the long run. For example, we wish for freedom to update 
exception messages in the future, while the added tests are restricting the 
messages our APIs can return.

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

Changes requested by liach (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28845#pullrequestreview-3600024617

Reply via email to