From 56c18e96ef00d67bd20e0ab188358b96e382511f Mon Sep 17 00:00:00 2001 From: Viktor Chynarov Date: Tue, 1 Oct 2019 11:08:42 -0700 Subject: [PATCH] Back out "Revert D17636918: [quic] Create findFrameInPacketFunc helper function for unit tests" Summary: Original commit changeset: dcc2b618c2a7 Reviewed By: lnicco Differential Revision: D17688819 fbshipit-source-id: 9dd3dbcfd78549f5858a41b67db7fc4bc0647469 --- quic/api/test/QuicTransportTest.cpp | 171 ++----------------- quic/common/test/TestUtils.h | 18 ++ quic/server/test/QuicServerTransportTest.cpp | 54 +----- 3 files changed, 35 insertions(+), 208 deletions(-) diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 9cfda0714..1eb8bdd2e 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -1026,22 +1026,7 @@ TEST_F(QuicTransportTest, ClonePathChallenge) { auto numPathChallengePackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](PathChallengeFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numPathChallengePackets, 1); // Force a timeout with no data so that it clones the packet @@ -1051,22 +1036,8 @@ TEST_F(QuicTransportTest, ClonePathChallenge) { numPathChallengePackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](PathChallengeFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); + EXPECT_EQ(numPathChallengePackets, 3); } @@ -1088,22 +1059,7 @@ TEST_F(QuicTransportTest, OnlyClonePathValidationIfOutstanding) { auto numPathChallengePackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](PathChallengeFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numPathChallengePackets, 1); // Reset outstandingPathValidation @@ -1116,22 +1072,7 @@ TEST_F(QuicTransportTest, OnlyClonePathValidationIfOutstanding) { numPathChallengePackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](PathChallengeFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numPathChallengePackets, 1); } @@ -1256,22 +1197,7 @@ TEST_F(QuicTransportTest, ClonePathResponse) { auto numPathResponsePackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](PathResponseFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numPathResponsePackets, 1); // Force a timeout with no data so that it clones the packet @@ -1279,22 +1205,7 @@ TEST_F(QuicTransportTest, ClonePathResponse) { numPathResponsePackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](PathResponseFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numPathResponsePackets, 1); } @@ -1365,22 +1276,7 @@ TEST_F(QuicTransportTest, CloneNewConnectionIdFrame) { auto numNewConnIdPackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](NewConnectionIdFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numNewConnIdPackets, 1); // Force a timeout with no data so that it clones the packet @@ -1390,22 +1286,7 @@ TEST_F(QuicTransportTest, CloneNewConnectionIdFrame) { numNewConnIdPackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](NewConnectionIdFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numNewConnIdPackets, 3); } @@ -1529,22 +1410,7 @@ TEST_F(QuicTransportTest, CloneRetireConnectionIdFrame) { auto numRetireConnIdPackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](RetireConnectionIdFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numRetireConnIdPackets, 1); // Force a timeout with no data so that it clones the packet @@ -1554,22 +1420,7 @@ TEST_F(QuicTransportTest, CloneRetireConnectionIdFrame) { numRetireConnIdPackets = std::count_if( conn.outstandingPackets.begin(), conn.outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](RetireConnectionIdFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); EXPECT_EQ(numRetireConnIdPackets, 3); } diff --git a/quic/common/test/TestUtils.h b/quic/common/test/TestUtils.h index 78630d3ab..1a1259d28 100644 --- a/quic/common/test/TestUtils.h +++ b/quic/common/test/TestUtils.h @@ -275,5 +275,23 @@ std::vector getQLogEventIndices( QLogEventType type, const std::shared_ptr& q); +template +auto findFrameInPacketFunc() { + return [&](auto& p) { + return std::find_if( + p.packet.frames.begin(), p.packet.frames.end(), [&](auto& f) { + return folly::variant_match( + f, + [&](QuicSimpleFrame& s) { + return folly::variant_match( + s, + [&](FrameType&) { return true; }, + [&](auto&) { return false; }); + }, + [&](auto&) { return false; }); + }) != p.packet.frames.end(); + }; +} + } // namespace test } // namespace quic diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 1b2bbb1db..a635d3c9b 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -1585,22 +1585,8 @@ TEST_F(QuicServerTransportTest, TestCloneStopSending) { auto packetItr = std::find_if( server->getNonConstConn().outstandingPackets.begin(), server->getNonConstConn().outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](StopSendingFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); + ASSERT_TRUE(packetItr != server->getNonConstConn().outstandingPackets.end()); // Force a timeout with no data so that it clones the packet server->lossTimeout().timeoutExpired(); @@ -1608,22 +1594,8 @@ TEST_F(QuicServerTransportTest, TestCloneStopSending) { auto numStopSendingPackets = std::count_if( server->getNonConstConn().outstandingPackets.begin(), server->getNonConstConn().outstandingPackets.end(), - [&](auto& p) { - return std::find_if( - p.packet.frames.begin(), - p.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](StopSendingFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != p.packet.frames.end(); - }); + findFrameInPacketFunc()); + EXPECT_GT(numStopSendingPackets, 1); std::vector indices = @@ -1639,22 +1611,8 @@ TEST_F(QuicServerTransportTest, TestAckStopSending) { server->getNonConstConn().streamManager->getStream(streamId); server->stopSending(streamId, GenericApplicationErrorCode::UNKNOWN); loopForWrites(); - auto match = [](OutstandingPacket& packet) { - return std::find_if( - packet.packet.frames.begin(), - packet.packet.frames.end(), - [&](auto& f) { - return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](StopSendingFrame&) { return true; }, - [&](auto&) { return false; }); - }, - [&](auto&) { return false; }); - }) != packet.packet.frames.end(); - }; + auto match = findFrameInPacketFunc(); + auto op = findOutstandingPacket(server->getNonConstConn(), match); ASSERT_TRUE(op != nullptr); PacketNum packetNum = op->packet.header.getPacketSequenceNum();