----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59850/#review177427 -----------------------------------------------------------
Fix it, then Ship it! I had a couple of minor comments about the tests, but otherwise it looks good. Oh, and don't forget to run Spotless! geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java Lines 82 (patched) <https://reviews.apache.org/r/59850/#comment250962> I'm missing the time delay here. Can you show me where it gets delayed? ... ah, it looks like you copied this from `TcpServerJUnitTest`. Would you mind renaming this function to something that makes more sense in this context? geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java Lines 104 (patched) <https://reviews.apache.org/r/59850/#comment250963> If we're expecting the exception as a requirement (and I think we are), we can assert that it's true by setting a boolean to false and then setting it to true in the catch block. geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java Lines 133 (patched) <https://reviews.apache.org/r/59850/#comment250978> We can reasonably set a very small timeout here, even less than the timeout of the socket (since we're mocking it). That (and setting the timeout to some silly number like 31337) would also help us verify that we're mocking correctly. geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java Lines 143 (patched) <https://reviews.apache.org/r/59850/#comment250964> What's the GemStoneAddition about? - Galen O'Sullivan On June 8, 2017, 12:20 a.m., Udo Kohlmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59850/ > ----------------------------------------------------------- > > (Updated June 8, 2017, 12:20 a.m.) > > > Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen > O'Sullivan, Hitesh Khamesra, and Brian Rowe. > > > Repository: geode > > > Description > ------- > > Moved the socket.setSoTimeout setting to be before the SSL handshake. This is > to avoid the timeout to never be set in the case of a SSLException. Added a > test to test that the socket timeout is correctly set upon failure within the > SSL configuration and handshake. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java > 86fe53261 > > geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/distributed/internal/tcpserver/TcpServerJUnitTest.java > 7c7a2b376 > > geode-core/src/test/java/org/apache/geode/internal/net/DelaySocketCreator.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/59850/diff/2/ > > > Testing > ------- > > Junit test - done > precheckin - done > > > Thanks, > > Udo Kohlmeyer > >
