RFR: 8343791: Socket.connect API should document whether the socket will be closed when hostname resolution fails or another error occurs [v12]

[Volkan Yazıcı]

On Thu, 21 Nov 2024 21:50:24 GMT, Mark Sheppard <msheppar at openjdk.org> wrote:

>> Volkan Yazıcı has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Handle `SocketTimeoutException` in `NioSocketImpl::connect()`
>
> test/jdk/java/net/Socket/CloseOnFailureTest.java line 128:
> 
>> 126:     @MethodSource("ctor_should_close_on_failures")
>> 127:     @SuppressWarnings("resource")
>> 128:     void ctor_should_close_on_failures(TestCase testCase) throws Throwable {
> 
> Is the style convention for  Java code  to use camel case for all classes, methods and variables ?

Switched back to camel-case in e37ee840b9681aa017f8653df68c69232a6b6c83. (Snake-case convention for test method names is something I adopted several years ago from other projects, since it was helping with making the failure output more human-readable. But I'd be more than happy to stick to the established convention in the code base.)

> test/jdk/java/net/Socket/CloseOnFailureTest.java line 149:
> 
>> 147: 
>> 148:     }
>> 149: 
> 
> static List<TestCase> ctorCloseOnFailuresTestCases() {
>          return List.of(
>                  TestCase.BindFailureFactory.ioExceptionTestCase(),
>                  TestCase.ConnectFailureFactory.ioExceptionTestCase(),
>                  TestCase.ConnectFailureFactory.illegalArgumentExceptionTestCase(1));
>      }
> 
> c.f. name change suggestion below

Applied this suggestion in e37ee840b9681aa017f8653df68c69232a6b6c83.

> test/jdk/java/net/Socket/CloseOnFailureTest.java line 210:
> 
>> 208:         private static final String ERROR_MESSAGE = "intentional test failure";
>> 209: 
>> 210:         private static final class ForBindFailure {
> 
> ForConnectionFailure ForBindfFailure are factories abstraction
> Their method are factory methods (the world is addicted to of methods) fabricating a TestCase
> 
> ofIOException
> ofIllegalArgumentException
> ofIOException() 
> 
> A simple change to ConnectionFailureFactory and BindFailureFactory
> With methods like ioExceptionTestCase illegalArgumentException
> 
> Lead to readable code like
> 
> TestCase.ConnectionFailureFactory.ioExceptionTestCase()
> 
> TestCase.BindFaiureFactory.illegalExceptionTestCase()

Applied your suggestions in e37ee840b9681aa017f8653df68c69232a6b6c83.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1853602370
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1853604475
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1853603172