From fa0aa9754e7e1984f39df0a6e458f0e087f0246f Mon Sep 17 00:00:00 2001 From: Brandon Schlinker Date: Sun, 12 Dec 2021 17:03:27 -0800 Subject: [PATCH] Fix open source build Summary: Fixing four issues in open source build. First, two missing changes to `CMakeLists.cpp` changes - `handshake/TokenGenerator.cpp`, added in D31673160 (https://github.com/facebookincubator/mvfst/commit/7233c55d29428cf9dccc944d3181be165a5b69e8) - `state/AckEvent.cpp`, added in D31221302 (https://github.com/facebookincubator/mvfst/commit/7a916869ff1795e38c2a60bc3a2f794b138ba33f) The two two issues are caused by inconsistencies between our build env and open source build env, including gtest versions. We should investigate getting open source gtest on same version. --- **[D31216724 (https://github.com/facebookincubator/mvfst/commit/20c17a882b7446d01f8387e90f0c8faa2a7b2385)] We cannot use lambdas with `testing::ResultOf` for older versions of gtest and/or older compilers.** ``` gmock-matchers.h:2310:29: error: no type named 'result_type' in '(lambda at /data/sandcastle/temp/fbcode_builder_getdeps/shipit/mvfst/quic/api/test/QuicTransportTest.cpp:711:19)' ``` In particular, the `decltype(f(arg))` logic in the following code from gtest is going to fail on the lambda, since a lambda has no type. I believe it would need to be a `declval(decltype(f(arg)))` for the lambda to work. ``` template struct CallableTraits { typedef Functor StorageType; static void CheckIsValid(Functor /* functor */) {} template static auto Invoke(Functor f, const T& arg) -> decltype(f(arg)) { return f(arg); } }; ``` See this for details: https://stackoverflow.com/questions/4846540/c11-lambda-in-decltype ----- **[D32772165 (https://github.com/facebookincubator/mvfst/commit/e784fafb1035f6fe90495868ec53ea0547914293)] `EXPECT_CALL` does not work with the formatting used for older versions of gtest and/or older compilers (again, unclear).** Specifically ``` EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished); ``` must instead be ``` EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished()); ``` Reviewed By: kvtsoy Differential Revision: D33026131 fbshipit-source-id: 886a6165f2e217cadbe479320195abbd4895eb7c --- quic/api/test/QuicTransportTest.cpp | 35 ++++++++++++-------- quic/server/CMakeLists.txt | 1 + quic/server/test/QuicServerTransportTest.cpp | 18 +++++----- quic/state/CMakeLists.txt | 1 + 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index bc6861130..0fa7e0647 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -702,22 +702,31 @@ TEST_F(QuicTransportTest, ObserverPacketsWrittenCycleCheckDetails) { // part 3.2, we write two ACK eliciting packets, then become app limited // one of the ACK eliciting packets contains an ACK frame { + // older versions of gtest do not seem to accept lambdas for ResultOf + // matcher, so define an std::function + std::function + countPacketsWithAckFrames = + [](const Observer::WriteEvent& event) -> uint64_t { + uint64_t packetsWithAckFrames = 0; + for (auto& outstandingPacket : event.outstandingPackets) { + bool hasAckFrame = false; + for (auto& frame : outstandingPacket.packet.frames) { + if (frame.asWriteAckFrame()) { + hasAckFrame = true; + } + } + + if (hasAckFrame) { + packetsWithAckFrames++; + } + } + return packetsWithAckFrames; + }; + const auto writeEventMatcher = AllOf( + testing::ResultOf(countPacketsWithAckFrames, 1), testing::Property( &Observer::WriteEvent::getOutstandingPackets, testing::SizeIs(9)), - testing::Property( - &Observer::WriteEvent::getOutstandingPackets, - testing::Contains(testing::ResultOf( - [](auto& outstandingPacket) { - for (auto& frame : outstandingPacket.packet.frames) { - auto ackFrame = frame.asWriteAckFrame(); - if (ackFrame) { - return true; - } - } - return false; - }, - testing::Eq(true)))), testing::Field( &Observer::WriteEvent::writeCount, testing::Eq(writeNum))); const auto packetsWrittenEventMatcher = AllOf( diff --git a/quic/server/CMakeLists.txt b/quic/server/CMakeLists.txt index 93678764b..1988c3ecd 100644 --- a/quic/server/CMakeLists.txt +++ b/quic/server/CMakeLists.txt @@ -8,6 +8,7 @@ add_library( handshake/AppToken.cpp handshake/ServerHandshake.cpp handshake/StatelessResetGenerator.cpp + handshake/TokenGenerator.cpp state/ServerStateMachine.cpp) target_include_directories( diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index a3b60d89c..7f115cf1b 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -3063,7 +3063,7 @@ TEST_F( folly::SocketAddress newPeer("100.101.102.103", 23456); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeUnfinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeUnfinished()); try { recvClientFinished(true, &newPeer); } catch (const std::runtime_error& ex) { @@ -3097,7 +3097,7 @@ TEST_F( auto data = IOBuf::copyBuffer("bad data"); folly::SocketAddress newPeer("100.101.102.103", 23456); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeUnfinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeUnfinished()); try { recvClientFinished(true, &newPeer); } catch (const std::runtime_error& ex) { @@ -3296,7 +3296,7 @@ TEST_F( *server->getNonConstConn().writableBytesLimit, server->getConn().transportSettings.limitedCwndInMss * originalUdpSize); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished()); recvClientFinished(); loopForWrites(); EXPECT_EQ(server->getConn().writableBytesLimit, folly::none); @@ -3317,7 +3317,7 @@ TEST_F( } TEST_F(QuicUnencryptedServerTransportTest, TestSendHandshakeDone) { - EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished()); getFakeHandshakeLayer()->allowZeroRttKeys(); setupClientReadCodec(); recvClientHello(true, QuicVersion::QUIC_DRAFT); @@ -3535,7 +3535,7 @@ TEST_F(QuicUnencryptedServerTransportTest, TestCloseWhileAsyncPending) { IOBufEqualTo eq; EXPECT_TRUE(eq(getCryptoStreamData(), IOBuf::copyBuffer("SHLO"))); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeUnfinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeUnfinished()); recvClientFinished(); server->close(std::make_pair( @@ -3661,7 +3661,7 @@ TEST_P( EXPECT_EQ(server->getConn().streamManager->streamCount(), 0); EXPECT_EQ(server->getConn().pendingOneRttData->size(), 1); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished()); recvClientFinished(); EXPECT_EQ(server->getConn().streamManager->streamCount(), 1); EXPECT_EQ(server->getConn().pendingZeroRttData, nullptr); @@ -3721,7 +3721,7 @@ TEST_P( GetParam().acceptZeroRtt ? 1 : 0); EXPECT_EQ(server->getConn().pendingOneRttData->size(), 1); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished()); recvClientFinished(); EXPECT_EQ( server->getConn().streamManager->streamCount(), @@ -3839,7 +3839,7 @@ class QuicServerTransportHandshakeTest EXPECT_CALL(routingCallback, onConnectionIdBound(_)); // NST is always written after CFIN is processed expectWriteNewSessionTicket(); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished()); recvClientFinished(); } @@ -3902,7 +3902,7 @@ TEST_P(QuicServerTransportHandshakeTest, TestD6DStartCallback) { TEST_F(QuicUnencryptedServerTransportTest, DuplicateOneRttWriteCipher) { setupClientReadCodec(); recvClientHello(); - EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished); + EXPECT_CALL(handshakeFinishedCallback, onHandshakeFinished()); recvClientFinished(); loopForWrites(); try { diff --git a/quic/state/CMakeLists.txt b/quic/state/CMakeLists.txt index 07ecb8e64..b6cceb0be 100644 --- a/quic/state/CMakeLists.txt +++ b/quic/state/CMakeLists.txt @@ -54,6 +54,7 @@ target_link_libraries( add_library( mvfst_state_ack_handler + AckEvent.cpp AckHandlers.cpp )