Github user pnowojski commented on a diff in the pull request:
https://github.com/apache/flink/pull/6355#discussion_r204330930
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/netty/NettyClientServerSslTest.java
---
@@ -65,6 +68,60 @@ public void testValidSslConnection() throws Exception {
Channel ch = NettyTestUtil.connect(serverAndClient);
+ SslHandler sslHandler = (SslHandler) ch.pipeline().get("ssl");
+ assertTrue("default value should not be propagated",
sslHandler.getHandshakeTimeoutMillis() >= 0);
+ assertTrue("default value should not be propagated",
sslHandler.getCloseNotifyTimeoutMillis() >= 0);
+
+ // should be able to send text data
+ ch.pipeline().addLast(new StringDecoder()).addLast(new
StringEncoder());
+ assertTrue(ch.writeAndFlush("test").await().isSuccess());
+
+ NettyTestUtil.shutdown(serverAndClient);
+ }
+
+ /**
+ * Verify valid (advanced) ssl configuration and connection.
+ */
+ @Test
+ public void testValidSslConnectionAdvanced() throws Exception {
--- End diff --
This is quite poor test :( With respect to `SESSION_CACHE_SIZE` and
`SESSION_TIMEOUT` it tests only for "not throwing any exception". If those
properties are just ignored, the test will still pass.
Can we add some stress test that actually verifies the bug which this PR is
trying to solve? Maybe stress test AND benchmark like
`StreamNetworkThroughputBenchmarkTest#largeRemoteMode`?
---
