Update tests to not use TestUtils.getFreePort() and so ensure we not …

…try to use a port that is used by the system in the meantime. Motivation: We should not try to detect a free port in tests put just use 0 when bind so there is no race in which the system my bind something to the port we choosen before. Modifications: - Remove the usage of TestUtils.getFreePort() in the testsuite - Remove hack to workaround bind errors which will not happen anymore now Result: Less flacky tests.
dschobel · Jul 20, 2017 · 3cdff36 · 3cdff36
1 parent e5a31a4
commit 3cdff36
Show file tree

Hide file tree

Showing 41 changed files with 98 additions and 233 deletions.
diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/sctp/AbstractSctpTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/sctp/AbstractSctpTest.java
@@ -21,14 +21,12 @@
 import io.netty.channel.ChannelOption;
 import io.netty.testsuite.transport.AbstractComboTestsuiteTest;
 import io.netty.testsuite.transport.TestsuitePermutation;
-import io.netty.testsuite.util.TestUtils;
 import io.netty.util.NetUtil;
 
 import java.net.InetSocketAddress;
 import java.util.List;
 
 public abstract class AbstractSctpTest extends AbstractComboTestsuiteTest<ServerBootstrap, Bootstrap> {
-    protected volatile InetSocketAddress addr;
 
     protected AbstractSctpTest() {
         super(ServerBootstrap.class, Bootstrap.class);
@@ -41,11 +39,9 @@ protected List<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Boots
 
     @Override
     protected void configure(ServerBootstrap serverBootstrap, Bootstrap bootstrap, ByteBufAllocator allocator) {
-        addr = new InetSocketAddress(NetUtil.LOCALHOST, TestUtils.getFreePort());
-        serverBootstrap.localAddress(addr);
+        serverBootstrap.localAddress(new InetSocketAddress(NetUtil.LOCALHOST, 0));
         serverBootstrap.option(ChannelOption.ALLOCATOR, allocator);
         serverBootstrap.childOption(ChannelOption.ALLOCATOR, allocator);
-        bootstrap.remoteAddress(addr);
         bootstrap.option(ChannelOption.ALLOCATOR, allocator);
     }
 }
diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/sctp/SctpEchoTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/sctp/SctpEchoTest.java
@@ -92,7 +92,7 @@ public void initChannel(SctpChannel c) throws Exception {
         });
 
         Channel sc = sb.bind().sync().channel();
-        Channel cc = cb.connect().sync().channel();
+        Channel cc = cb.connect(sc.localAddress()).sync().channel();
 
         for (int i = 0; i < data.length;) {
             int length = Math.min(random.nextInt(1024 * 64), data.length - i);

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractClientSocketTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractClientSocketTest.java
@@ -20,7 +20,6 @@
 import io.netty.channel.ChannelOption;
 import io.netty.testsuite.transport.AbstractTestsuiteTest;
 import io.netty.testsuite.transport.TestsuitePermutation;
-import io.netty.testsuite.util.TestUtils;
 import io.netty.util.NetUtil;
 
 import java.net.InetSocketAddress;
@@ -29,8 +28,6 @@
 
 public abstract class AbstractClientSocketTest extends AbstractTestsuiteTest<Bootstrap> {
 
-    protected volatile SocketAddress addr;
-
     protected AbstractClientSocketTest() {
         super(Bootstrap.class);
     }
@@ -42,13 +39,10 @@ protected List<TestsuitePermutation.BootstrapFactory<Bootstrap>> newFactories()
 
     @Override
     protected void configure(Bootstrap bootstrap, ByteBufAllocator allocator) {
-        addr = newSocketAddress();
-        bootstrap.remoteAddress(addr);
         bootstrap.option(ChannelOption.ALLOCATOR, allocator);
     }
 
     protected SocketAddress newSocketAddress() {
-        return new InetSocketAddress(
-                NetUtil.LOCALHOST, TestUtils.getFreePort());
+        return new InetSocketAddress(NetUtil.LOCALHOST, 0);
     }
 }
diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractDatagramTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractDatagramTest.java
@@ -20,16 +20,14 @@
 import io.netty.channel.ChannelOption;
 import io.netty.testsuite.transport.AbstractComboTestsuiteTest;
 import io.netty.testsuite.transport.TestsuitePermutation;
-import io.netty.testsuite.util.TestUtils;
 import io.netty.util.NetUtil;
 
 import java.net.InetSocketAddress;
+import java.net.SocketAddress;
 import java.util.List;
 
 public abstract class AbstractDatagramTest extends AbstractComboTestsuiteTest<Bootstrap, Bootstrap> {
 
-    protected volatile InetSocketAddress addr;
-
     protected AbstractDatagramTest() {
         super(Bootstrap.class, Bootstrap.class);
     }
@@ -41,16 +39,16 @@ protected List<TestsuitePermutation.BootstrapComboFactory<Bootstrap, Bootstrap>>
 
     @Override
     protected void configure(Bootstrap bootstrap, Bootstrap bootstrap2, ByteBufAllocator allocator) {
-        addr = new InetSocketAddress(
-                NetUtil.LOCALHOST4, TestUtils.getFreePort());
-        bootstrap.localAddress(addr);
         bootstrap.option(ChannelOption.ALLOCATOR, allocator);
-        bootstrap2.localAddress(0).remoteAddress(addr);
         bootstrap2.option(ChannelOption.ALLOCATOR, allocator);
     }
 
-    protected void refreshLocalAddress(Bootstrap bootstrap) {
-        addr = new InetSocketAddress(NetUtil.LOCALHOST4, TestUtils.getFreePort());
-        bootstrap.localAddress(addr);
+    protected SocketAddress newSocketAddress() {
+        // We use LOCALHOST4 as we use InternetProtocolFamily.IPv4 when creating the DatagramChannel and its
+        // not supported to bind to and IPV6 address in this case.
+        //
+        // See also http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/e74259b3eadc/
+        // src/share/classes/sun/nio/ch/DatagramChannelImpl.java#l684
+        return new InetSocketAddress(NetUtil.LOCALHOST4, 0);
     }
 }
diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractServerSocketTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractServerSocketTest.java
@@ -20,7 +20,6 @@
 import io.netty.channel.ChannelOption;
 import io.netty.testsuite.transport.AbstractTestsuiteTest;
 import io.netty.testsuite.transport.TestsuitePermutation;
-import io.netty.testsuite.util.TestUtils;
 import io.netty.util.NetUtil;
 
 import java.net.InetSocketAddress;
@@ -29,8 +28,6 @@
 
 public abstract class AbstractServerSocketTest extends AbstractTestsuiteTest<ServerBootstrap> {
 
-    protected volatile SocketAddress addr;
-
     protected AbstractServerSocketTest() {
         super(ServerBootstrap.class);
     }
@@ -42,14 +39,13 @@ protected List<TestsuitePermutation.BootstrapFactory<ServerBootstrap>> newFactor
 
     @Override
     protected void configure(ServerBootstrap bootstrap, ByteBufAllocator allocator) {
-        addr = newSocketAddress();
-        bootstrap.localAddress(addr);
+        bootstrap.localAddress(newSocketAddress());
         bootstrap.option(ChannelOption.ALLOCATOR, allocator);
         bootstrap.childOption(ChannelOption.ALLOCATOR, allocator);
     }
 
     protected SocketAddress newSocketAddress() {
         return new InetSocketAddress(
-                NetUtil.LOCALHOST, TestUtils.getFreePort());
+                NetUtil.LOCALHOST, 0);
     }
 }
diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractSocketTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/AbstractSocketTest.java
@@ -21,7 +21,6 @@
 import io.netty.channel.ChannelOption;
 import io.netty.testsuite.transport.AbstractComboTestsuiteTest;
 import io.netty.testsuite.transport.TestsuitePermutation;
-import io.netty.testsuite.util.TestUtils;
 import io.netty.util.NetUtil;
 
 import java.net.InetSocketAddress;
@@ -30,8 +29,6 @@
 
 public abstract class AbstractSocketTest extends AbstractComboTestsuiteTest<ServerBootstrap, Bootstrap> {
 
-    protected volatile SocketAddress addr;
-
     protected AbstractSocketTest() {
         super(ServerBootstrap.class, Bootstrap.class);
     }
@@ -43,16 +40,13 @@ protected List<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Boots
 
     @Override
     protected void configure(ServerBootstrap bootstrap, Bootstrap bootstrap2, ByteBufAllocator allocator) {
-        addr = newSocketAddress();
-        bootstrap.localAddress(addr);
+        bootstrap.localAddress(newSocketAddress());
         bootstrap.option(ChannelOption.ALLOCATOR, allocator);
         bootstrap.childOption(ChannelOption.ALLOCATOR, allocator);
-        bootstrap2.remoteAddress(addr);
         bootstrap2.option(ChannelOption.ALLOCATOR, allocator);
     }
 
     protected SocketAddress newSocketAddress() {
-        return new InetSocketAddress(
-                NetUtil.LOCALHOST, TestUtils.getFreePort());
+        return new InetSocketAddress(NetUtil.LOCALHOST, 0);
     }
 }
diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramMulticastTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramMulticastTest.java
@@ -57,9 +57,12 @@ public void channelRead0(ChannelHandlerContext ctx, Object msg) throws Exception
         sb.option(ChannelOption.SO_REUSEADDR, true);
         cb.option(ChannelOption.IP_MULTICAST_IF, NetUtil.LOOPBACK_IF);
         cb.option(ChannelOption.SO_REUSEADDR, true);
+
+        Channel sc = sb.bind(newSocketAddress()).sync().channel();
+
+        InetSocketAddress addr = (InetSocketAddress) sc.localAddress();
         cb.localAddress(addr.getPort());
 
-        Channel sc = sb.bind().sync().channel();
         if (sc instanceof OioDatagramChannel) {
             // skip the test for OIO, as it fails because of
             // No route to host which makes no sense.

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramUnicastTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramUnicastTest.java
@@ -27,7 +27,7 @@
 import io.netty.channel.socket.DatagramPacket;
 import org.junit.Test;
 
-import java.net.BindException;
+import java.net.InetSocketAddress;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
@@ -159,35 +159,16 @@ public void channelRead0(ChannelHandlerContext ctx, Object msgs) throws Exceptio
             }
         });
 
-        Channel sc = null;
-        BindException bindFailureCause = null;
-        for (int i = 0; i < 3; i ++) {
-            try {
-                sc = sb.bind().sync().channel();
-                break;
-            } catch (Exception e) {
-                if (e instanceof BindException) {
-                    logger.warn("Failed to bind to a free port; trying again", e);
-                    bindFailureCause = (BindException) e;
-                    refreshLocalAddress(sb);
-                } else {
-                    throw e;
-                }
-            }
-        }
-
-        if (sc == null) {
-            throw bindFailureCause;
-        }
-
+        Channel sc = sb.bind(newSocketAddress()).sync().channel();
         Channel cc;
         if (bindClient) {
-            cc = cb.bind().sync().channel();
+            cc = cb.bind(newSocketAddress()).sync().channel();
         } else {
             cb.option(ChannelOption.DATAGRAM_CHANNEL_ACTIVE_ON_REGISTRATION, true);
             cc = cb.register().sync().channel();
         }
 
+        InetSocketAddress addr = (InetSocketAddress) sc.localAddress();
         for (int i = 0; i < count; i++) {
             switch (wrapType) {
                 case DUP:

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/ServerSocketSuspendTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/ServerSocketSuspendTest.java
@@ -58,7 +58,7 @@ public void testSuspendAndResumeAccept(ServerBootstrap sb) throws Throwable {
             long startTime = System.nanoTime();
             for (int i = 0; i < NUM_CHANNELS; i ++) {
                 Socket s = new Socket();
-                SocketUtils.connect(s, addr, 10000);
+                SocketUtils.connect(s, sc.localAddress(), 10000);
                 sockets.add(s);
             }
 
@@ -80,7 +80,7 @@ public void testSuspendAndResumeAccept(ServerBootstrap sb) throws Throwable {
             long startTime = System.nanoTime();
             for (int i = 0; i < NUM_CHANNELS; i ++) {
                 Socket s = new Socket();
-                s.connect(addr, 10000);
+                s.connect(sc.localAddress(), 10000);
                 sockets.add(s);
             }
             long endTime = System.nanoTime();

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketAutoReadTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketAutoReadTest.java
@@ -66,14 +66,13 @@ private static void testAutoReadOffDuringReadOnlyReadsOneTime(boolean readOutsid
 
             serverChannel = sb.bind().syncUninterruptibly().channel();
 
-            cb.remoteAddress(serverChannel.localAddress())
-                    .option(ChannelOption.AUTO_READ, true)
+            cb.option(ChannelOption.AUTO_READ, true)
                     // We want to ensure that we attempt multiple individual read operations per read loop so we can
                     // test the auto read feature being turned off when data is first read.
                     .option(ChannelOption.RCVBUF_ALLOCATOR, new TestRecvByteBufAllocator())
                     .handler(clientInitializer);
 
-            clientChannel = cb.connect().syncUninterruptibly().channel();
+            clientChannel = cb.connect(serverChannel.localAddress()).syncUninterruptibly().channel();
 
             // 3 bytes means 3 independent reads for TestRecvByteBufAllocator
             clientChannel.writeAndFlush(Unpooled.wrappedBuffer(new byte[3]));

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketBufReleaseTest.java
@@ -53,7 +53,7 @@ public void testBufRelease(ServerBootstrap sb, Bootstrap cb) throws Throwable {
         cb.handler(clientHandler);
 
         Channel sc = sb.bind().sync().channel();
-        Channel cc = cb.connect().sync().channel();
+        Channel cc = cb.connect(sc.localAddress()).sync().channel();
 
         // Ensure the server socket accepted the client connection *and* initialized pipeline successfully.
         serverHandler.channelFuture.sync();

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketCancelWriteTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketCancelWriteTest.java
@@ -51,7 +51,7 @@ public void testCancelWrite(ServerBootstrap sb, Bootstrap cb) throws Throwable {
         sb.childHandler(sh);
 
         Channel sc = sb.bind().sync().channel();
-        Channel cc = cb.connect().sync().channel();
+        Channel cc = cb.connect(sc.localAddress()).sync().channel();
 
         ChannelFuture f = cc.write(a);
         assertTrue(f.cancel(false));

diff --git a/...e/src/main/java/io/netty/testsuite/transport/socket/SocketChannelNotYetConnectedTest.java b/...e/src/main/java/io/netty/testsuite/transport/socket/SocketChannelNotYetConnectedTest.java
@@ -20,7 +20,6 @@
 import io.netty.channel.socket.SocketChannel;
 import org.junit.Test;
 
-import java.net.InetSocketAddress;
 import java.net.SocketException;
 import java.nio.channels.NotYetConnectedException;
 
@@ -34,7 +33,7 @@ public void testShutdownNotYetConnected() throws Throwable {
 
     public void testShutdownNotYetConnected(Bootstrap cb) throws Throwable {
         SocketChannel ch = (SocketChannel) cb.handler(new ChannelInboundHandlerAdapter())
-                .bind(new InetSocketAddress(0)).syncUninterruptibly().channel();
+                .bind(newSocketAddress()).syncUninterruptibly().channel();
         try {
             try {
                 ch.shutdownInput().syncUninterruptibly();

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketCloseForciblyTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketCloseForciblyTest.java
@@ -44,7 +44,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
 
         Channel sc = sb.bind().sync().channel();
 
-        cb.connect().channel().closeFuture().syncUninterruptibly();
+        cb.connect(sc.localAddress()).channel().closeFuture().syncUninterruptibly();
         sc.close().sync();
     }
 }
diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectTest.java
@@ -84,7 +84,7 @@ public void testChannelEventsFiredWhenClosedDirectly(ServerBootstrap sb, Bootstr
         Channel cc = null;
         try {
             sb.childHandler(new ChannelInboundHandlerAdapter());
-            sc = sb.bind(NetUtil.LOCALHOST, TestUtils.getFreePort()).syncUninterruptibly().channel();
+            sc = sb.bind().syncUninterruptibly().channel();
 
             cb.handler(new ChannelInboundHandlerAdapter() {
                 @Override

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectionAttemptTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectionAttemptTest.java
@@ -23,7 +23,6 @@
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.ChannelOption;
 import io.netty.util.internal.SocketUtils;
-import io.netty.testsuite.util.TestUtils;
 import io.netty.util.NetUtil;
 import io.netty.util.concurrent.GlobalEventExecutor;
 import io.netty.util.concurrent.Promise;
@@ -45,6 +44,9 @@ public class SocketConnectionAttemptTest extends AbstractClientSocketTest {
     private static final String BAD_HOST = SystemPropertyUtil.get("io.netty.testsuite.badHost", "netty.io");
     private static final int BAD_PORT = SystemPropertyUtil.getInt("io.netty.testsuite.badPort", 65535);
 
+    // See /etc/services
+    private static final int UNASSIGNED_PORT = 4;
+
     static {
         InternalLogger logger = InternalLoggerFactory.getInstance(SocketConnectionAttemptTest.class);
         logger.debug("-Dio.netty.testsuite.badHost: {}", BAD_HOST);
@@ -89,14 +91,13 @@ private static void testConnectRefused0(Bootstrap cb, boolean halfClosure) throw
         ChannelHandler handler = new ChannelInboundHandlerAdapter() {
             @Override
             public void channelActive(ChannelHandlerContext ctx) throws Exception {
-                Channel channel = ctx.channel();
                 errorPromise.setFailure(new AssertionError("should have never been called"));
             }
         };
 
         cb.handler(handler);
         cb.option(ChannelOption.ALLOW_HALF_CLOSURE, halfClosure);
-        ChannelFuture future = cb.connect(NetUtil.LOCALHOST, TestUtils.getFreePort()).awaitUninterruptibly();
+        ChannelFuture future = cb.connect(NetUtil.LOCALHOST, UNASSIGNED_PORT).awaitUninterruptibly();
         assertThat(future.cause(), is(instanceOf(ConnectException.class)));
         assertThat(errorPromise.cause(), is(nullValue()));
     }

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketEchoTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketEchoTest.java
@@ -155,7 +155,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
         cb.option(ChannelOption.AUTO_READ, autoRead);
 
         Channel sc = sb.bind().sync().channel();
-        Channel cc = cb.connect().sync().channel();
+        Channel cc = cb.connect(sc.localAddress()).sync().channel();
 
         for (int i = 0; i < data.length;) {
             int length = Math.min(random.nextInt(1024 * 64), data.length - i);

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketExceptionHandlingTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketExceptionHandlingTest.java
@@ -50,9 +50,8 @@ public void testReadPendingIsResetAfterEachRead(ServerBootstrap sb, Bootstrap cb
 
             serverChannel = sb.bind().syncUninterruptibly().channel();
 
-            cb.remoteAddress(serverChannel.localAddress())
-              .handler(new MyInitializer());
-            clientChannel = cb.connect().syncUninterruptibly().channel();
+            cb.handler(new MyInitializer());
+            clientChannel = cb.connect(serverChannel.localAddress()).syncUninterruptibly().channel();
 
             clientChannel.writeAndFlush(Unpooled.wrappedBuffer(new byte[1024]));
 

diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketFileRegionTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketFileRegionTest.java
@@ -146,7 +146,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
 
         Channel sc = sb.bind().sync().channel();
 
-        Channel cc = cb.connect().sync().channel();
+        Channel cc = cb.connect(sc.localAddress()).sync().channel();
         FileRegion region = new DefaultFileRegion(
                 new FileInputStream(file).getChannel(), startOffset, data.length - bufferSize);
         FileRegion emptyRegion = new DefaultFileRegion(new FileInputStream(file).getChannel(), 0, 0);