On Fri, 10 Apr 2026 21:14:56 GMT, Marcono1234 <[email protected]> wrote:

>> Hi, may I get a review for this change that converts the remaining TestNG 
>> tests under test/jdk/java/net to JUnit.
>> 
>> An exception is `java/net/NetworkInterface/NetworkInterfaceStreamTest.java` 
>> which depends on library classes depending on TestNG. Converting that test 
>> is not done here but is tracked by 
>> [JDK-8381848](https://bugs.openjdk.org/browse/JDK-8381848)
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> test/jdk/java/net/Socks/SocksSocketImplTest.java line 83:
> 
>> 81:             // expected
>> 82:             // now verify the IOE was thrown for the correct expected 
>> reason
>> 83:             if (!(ioe.getCause() instanceof IllegalArgumentException)) {
> 
> Use `assertThrows`? E.g.:
> 
> var e = assertThrows(IOException.class, ...);
> assertInstanceOf(IllegalArgumentException.class, e.getCause());
> 
> 
> That would also make it clearer which of the calls is expected to throw the 
> `IOException`.

Ok for `assertThrows` - but I will not use `assertInstanceOf` in this case: we 
want to throw the parent exception and not just the cause.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30681#discussion_r3073294258

Reply via email to