Repository: netty/netty
Issue: TlsReadTest tests are misleading
Labels: defect
improvement

Tests rely on channel having AutoRead = false and verify that read was issued through setting readIssued. In reality read method is called during construction of EmbeddedChannel - because AutoRead is true at this time, before AutoRead is set to false in test.

Issue comments:
@nayato can you submit a pr with something that you think makes things more clear ?
@nayato - please check if https://github.com/netty/netty/pull/6982 is what you had in mind.
@nayato does https://github.com/netty/netty/pull/6987 work for you ?
Fixed by https://github.com/netty/netty/pull/6987

Pull request review comments:

Patch that resolved the issue:
diff --git a/transport/src/main/java/io/netty/channel/embedded/EmbeddedChannel.java b/transport/src/main/java/io/netty/channel/embedded/EmbeddedChannel.java
index 2dcb577deb7..58302c60bbb 100644
--- a/transport/src/main/java/io/netty/channel/embedded/EmbeddedChannel.java
+++ b/transport/src/main/java/io/netty/channel/embedded/EmbeddedChannel.java
@@ -18,6 +18,7 @@
 import java.net.SocketAddress;
 import java.nio.channels.ClosedChannelException;
 import java.util.ArrayDeque;
+import java.util.Map;
 import java.util.Queue;
 
 import io.netty.channel.AbstractChannel;
@@ -29,6 +30,7 @@
 import io.netty.channel.ChannelId;
 import io.netty.channel.ChannelInitializer;
 import io.netty.channel.ChannelMetadata;
+import io.netty.channel.ChannelOption;
 import io.netty.channel.ChannelOutboundBuffer;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.ChannelPromise;
@@ -121,6 +123,20 @@ public EmbeddedChannel(ChannelId channelId, final ChannelHandler... handlers) {
         this(channelId, false, handlers);
     }
 
+    /**
+     * Create a new instance with the channel options applied and the pipeline initialized with the specified handlers.
+     *
+     * @param options {@link ChannelOption}s that will be applied before the handlers are initialized.
+     * @param handlers the {@link ChannelHandler}s which will be add in the {@link ChannelPipeline}
+     */
+    public EmbeddedChannel(Map<ChannelOption<?>, ?> options, final ChannelHandler... handlers) {
+        super(null, EmbeddedChannelId.INSTANCE);
+        metadata = metadata(false);
+        config = new DefaultChannelConfig(this);
+        this.config.setOptions(options);
+        setup(handlers);
+    }
+
     /**
      * Create a new instance with the channel ID set to the given ID and the pipeline
      * initialized with the specified handlers.


Test patch:
diff --git a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
index 84559a76407..d67986abfca 100644
--- a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
@@ -27,6 +27,7 @@
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.ChannelInitializer;
+import io.netty.channel.ChannelOption;
 import io.netty.channel.ChannelOutboundHandlerAdapter;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.embedded.EmbeddedChannel;
@@ -55,10 +56,13 @@
 import java.security.KeyStore;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
+
 import javax.net.ssl.ManagerFactoryParameters;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
@@ -160,28 +164,32 @@ public void read(ChannelHandlerContext ctx) throws Exception {
         }
 
         public void test(final boolean dropChannelActive) throws Exception {
-          SSLEngine engine = SSLContext.getDefault().createSSLEngine();
-          engine.setUseClientMode(true);
-
-          EmbeddedChannel ch = new EmbeddedChannel(
-              this,
-              new SslHandler(engine),
-              new ChannelInboundHandlerAdapter() {
-                @Override
-                public void channelActive(ChannelHandlerContext ctx) throws Exception {
-                  if (!dropChannelActive) {
-                    ctx.fireChannelActive();
-                  }
-                }
-              }
-          );
-          ch.config().setAutoRead(false);
-          assertFalse(ch.config().isAutoRead());
-
-          assertTrue(ch.writeOutbound(Unpooled.EMPTY_BUFFER));
-          assertTrue(readIssued);
-          assertTrue(ch.finishAndReleaseAll());
-       }
+            SSLEngine engine = SSLContext.getDefault().createSSLEngine();
+            engine.setUseClientMode(true);
+
+            Map<ChannelOption<?>, Object> options = new HashMap<ChannelOption<?>, Object>();
+            options.put(ChannelOption.AUTO_READ, false);
+            EmbeddedChannel ch = new EmbeddedChannel(
+                    options,
+                    this,
+                    new SslHandler(engine),
+                    new ChannelInboundHandlerAdapter() {
+                        @Override
+                        public void channelActive(ChannelHandlerContext ctx) throws Exception {
+                            if (!dropChannelActive) {
+                                ctx.fireChannelActive();
+                            }
+                        }
+                    }
+            );
+            assertFalse(ch.config().isAutoRead());
+            assertTrue(readIssued);
+            readIssued = false;
+
+            assertTrue(ch.writeOutbound(Unpooled.EMPTY_BUFFER));
+            assertTrue(readIssued);
+            assertTrue(ch.finishAndReleaseAll());
+        }
     }
 
     @Test