RFR 15 8243099: SO_INCOMING_NAPI_ID support

Daniel Fuchs daniel.fuchs at oracle.com
Thu May 14 09:37:14 UTC 2020


Hi Vladimir, Patrick,

The new tests look very good. There's just a small issue I noticed.
It's present in all the new tests - here is an example on 
AsynchronousSocketChannelNAPITest.java:

This code in @BeforeTest:

   63         try (var s = AsynchronousSocketChannel.open()) {
   64             if (!s.supportedOptions().contains(SO_INCOMING_NAPI_ID))
   65                 throw new SkippedException("NAPI ID not supported 
on this system");
   66         }

will prevent this code in @Test

   77             } else {
   78                 assertThrows(UOE, () -> 
sc.getOption(SO_INCOMING_NAPI_ID));
   79                 assertThrows(UOE, () -> 
sc.setOption(SO_INCOMING_NAPI_ID, 42));
   80                 assertThrows(UOE, () -> 
sc.setOption(SO_INCOMING_NAPI_ID, null));
   81             }


from ever being run. So I would suggest moving that check (lines 77-81
and 92-96) into the @BeforeTest method as below, and simply assume
that the option is supported when you reach the @Test:

   60     @BeforeTest
   61     public void setup() throws IOException {
   62         IPSupport.throwSkippedExceptionIfNonOperational();
   63         try (var s = AsynchronousSocketChannel.open()) {
                  if (!s.supportedOptions().contains(SO_INCOMING_NAPI_ID)) {
                      assertThrows(UOE, () -> 
sc.getOption(SO_INCOMING_NAPI_ID));
                      assertThrows(UOE, () -> 
sc.setOption(SO_INCOMING_NAPI_ID, 42));
                      assertThrows(UOE, () -> 
sc.setOption(SO_INCOMING_NAPI_ID, null));

   65                 throw new SkippedException("NAPI ID not supported 
on this system");
   66         }
   67         hostAddr = InetAddress.getLocalHost();
   68     }

Then the @Test can be simplified as follows:

   70     @Test
   71     public void testSetGetOptionSocketChannel() throws IOException {
   72         try (var sc = AsynchronousSocketChannel.open()) {
                  assertEquals((int) sc.getOption(SO_INCOMING_NAPI_ID), 0);
                  assertThrows(SE, () -> 
sc.setOption(SO_INCOMING_NAPI_ID, 42));
                  assertThrows(IAE, () -> 
sc.setOption(SO_INCOMING_NAPI_ID, null));
   82         }
   83     }

best regards,

-- daniel

On 13/05/2020 21:50, Ivanov, Vladimir A wrote:
> Thanks a lot. The updated webrev available as http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.13/
> 
> Thanks, Vladimir


More information about the net-dev mailing list