flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pnowojski <...@git.apache.org>
Subject [GitHub] flink pull request #6355: [FLINK-9878][network][ssl] add more low-level ssl ...
Date Mon, 23 Jul 2018 09:29:34 GMT
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`?


---

Mime
View raw message