On Wed, 6 Nov 2024 07:31:37 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> Tim Prinzing has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   suggested changes
>
> src/jdk.jfr/share/classes/jdk/jfr/internal/JDKEvents.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> I assume you didn't mean to change that.

Not sure how that happened, but I'll fix it

> test/jdk/jdk/jfr/event/io/TestSocketChannelEvents.java line 106:
> 
>> 104: 
>> 105:                 try (SocketChannel sc = 
>> SocketChannel.open(ssc.getLocalAddress())) {
>> 106:                     
>> addExpectedEvent(IOEvent.createSocketConnectEvent(sc.socket()));
> 
> This is SocketChannel in blocking mode where the connect succeeds. There is 
> also the non-blocking and where connect fails. In addition the connection can 
> established with the socket adaptor. So 6 possible cases for SocketChannel if 
> the test is expanded.

yes, testing still to be done

> test/jdk/jdk/jfr/event/io/TestSocketEvents.java line 108:
> 
>> 106:                 try (Socket s = new Socket()) {
>> 107:                     s.connect(ss.getLocalSocketAddress());
>> 108:                     
>> addExpectedEvent(IOEvent.createSocketConnectEvent(s));
> 
> This is Socket.connect success case, I assume we'll need a test for fail case 
> too.

yes

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1831527507
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1831529879
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1831529362

Reply via email to