> On May 16, 2016, 4:43 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/src/tests/ssl_tests.cpp, lines 730-731 > > <https://reviews.apache.org/r/47362/diff/1/?file=1383164#file1383164line730> > > > > IIUC, this is an implementation detail, which may change in the future. > > Currently our code relies on the callback from lebevent, which is invoked > > then data is sent. We actually don't care whether the SSL handshake has > > completed, we *just* want that a pending socket does not prevent us from > > accepting new connections. > > > > How about > > ``` > > // We initiate a connection on which we will not send > > // any data. This may prevent the socket from entering > > // ready state and hence the future signaled. > > ```
Hm.. not sure I follow your thoughts here. Libevent is the implementation detail, SSL handshake is an implementation detail of SSL, but it's an important aspect of this test. > we just want that a pending socket does not prevent us from accepting new > connections. What does "pending" mean or "ready" state mean? Sockets in themselves do not have a notion of "pending" or "ready" states, these words are revealing the notion of SSL handshaking / downgrading. Since we're testing SSL sockets here, it's helpful to discuss the aspects of SSL that we're testing, no? > On May 16, 2016, 4:43 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/src/tests/ssl_tests.cpp, lines 755-759 > > <https://reviews.apache.org/r/47362/diff/1/?file=1383164#file1383164line755> > > > > Why do you construct the response manually instead of using `http::OK`? The first is consistency with the rest of the tests here, but I did look at whether I could use http::OK. The wrinkle is that we have to directly send the bytes on the socket and so we have to use the encoder. The response encoder requires the corresponding request object, which we don't have here (I'd have to make this test more inconsistent with the rest). - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47362/#review133375 ----------------------------------------------------------- On May 13, 2016, 9:46 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47362/ > ----------------------------------------------------------- > > (Updated May 13, 2016, 9:46 p.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and Neil > Conway. > > > Bugs: MESOS-5340 > https://issues.apache.org/jira/browse/MESOS-5340 > > > Repository: mesos > > > Description > ------- > > This test works by creating a connection on which no data is sent (the SSL > handshake does not complete, nor is the socket downgraded). After which, we > expect that an HTTP request should succeed. > > > Diffs > ----- > > 3rdparty/libprocess/src/tests/ssl_tests.cpp > 4d237815a03828b915e821c3af78132e2915c610 > > Diff: https://reviews.apache.org/r/47362/diff/ > > > Testing > ------- > > This test fails before the fix in MESOS-5340. > > ``` > [==========] Running 1 test from 1 test case. > [----------] Global test environment set-up. > [----------] 1 test from SSLTest > [ RUN ] SSLTest.SilentSocket > ../../../3rdparty/libprocess/src/tests/ssl_tests.cpp:752: Failure > Failed to wait 15secs for socket > [ FAILED ] SSLTest.SilentSocket (15221 ms) > [----------] 1 test from SSLTest (15221 ms total) > > [----------] Global test environment tear-down > [==========] 1 test from 1 test case ran. (15222 ms total) > [ PASSED ] 0 tests. > [ FAILED ] 1 test, listed below: > [ FAILED ] SSLTest.SilentSocket > ``` > > > Thanks, > > Benjamin Mahler > >
