On Wed, 17 Dec 2025 22:43:44 GMT, Chen Liang <[email protected]> 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
