From c298c3c62e8b7f683f21cc97b2f55fdc710d12df Mon Sep 17 00:00:00 2001 From: Jeff Feasel Date: Sat, 4 May 2024 10:07:36 -0400 Subject: [PATCH 1/5] Refactored TestTCP and TestUDP to simplify them. Pulled out the places where test needs access to private members and put those in a TestAccess class. Moved TCPBaseTestAccess into an H file so it can be used by other tests in the future. --- src/inet/TCPEndPoint.h | 5 - src/transport/raw/TCP.h | 7 +- src/transport/raw/tests/TCPBaseTestAccess.h | 51 ++ src/transport/raw/tests/TestMessageHeader.cpp | 3 +- src/transport/raw/tests/TestPeerAddress.cpp | 3 +- src/transport/raw/tests/TestTCP.cpp | 491 +++++++++--------- src/transport/raw/tests/TestUDP.cpp | 111 ++-- 7 files changed, 373 insertions(+), 298 deletions(-) create mode 100644 src/transport/raw/tests/TCPBaseTestAccess.h diff --git a/src/inet/TCPEndPoint.h b/src/inet/TCPEndPoint.h index 0c00e327cc3d71..8fc6a6338d4140 100644 --- a/src/inet/TCPEndPoint.h +++ b/src/inet/TCPEndPoint.h @@ -38,10 +38,6 @@ namespace chip { -namespace Transport { -class TCPTest; -}; - namespace Inet { class TCPTest; @@ -529,7 +525,6 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis constexpr static size_t kMaxReceiveMessageSize = System::PacketBuffer::kMaxSizeWithoutReserve; protected: - friend class ::chip::Transport::TCPTest; friend class TCPTest; TCPEndPoint(EndPointManager & endPointManager) : diff --git a/src/transport/raw/TCP.h b/src/transport/raw/TCP.h index bb4671215b96c8..5016157a08c985 100644 --- a/src/transport/raw/TCP.h +++ b/src/transport/raw/TCP.h @@ -41,6 +41,9 @@ namespace chip { namespace Transport { +// Forward declaration of friend class for test access. +template class TCPBaseTestAccess; + /** Defines listening parameters for setting up a TCP transport */ class TcpListenParameters { @@ -200,7 +203,8 @@ class DLL_EXPORT TCPBase : public Base void CloseActiveConnections(); private: - friend class TCPTest; + // Allow tests to access private members. + template friend class TCPBaseTestAccess; /** * Allocate an unused connection from the pool @@ -330,7 +334,6 @@ class TCP : public TCPBase ~TCP() override { mPendingPackets.ReleaseAll(); } private: - friend class TCPTest; ActiveTCPConnectionState mConnectionsBuffer[kActiveConnectionsSize]; PoolImpl mPendingPackets; }; diff --git a/src/transport/raw/tests/TCPBaseTestAccess.h b/src/transport/raw/tests/TCPBaseTestAccess.h new file mode 100644 index 00000000000000..181583ecedd5f9 --- /dev/null +++ b/src/transport/raw/tests/TCPBaseTestAccess.h @@ -0,0 +1,51 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#if INET_CONFIG_ENABLE_TCP_ENDPOINT +#include +#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT + +using namespace chip::Inet; + +namespace chip { +namespace Transport { +/** + * @brief Class acts as an accessor to private members of the TCPBase class without needing to give + * friend access to each individual test. + */ +template +class TCPBaseTestAccess +{ +public: + using TCPImpl = Transport::TCP; + + static void * FindActiveConnection(TCPImpl & tcp, Transport::PeerAddress & peerAddress) + { + return tcp.FindActiveConnection(peerAddress); + } + static TCPEndPoint * GetEndpoint(void * state) { return static_cast(state)->mEndPoint; } + + static CHIP_ERROR ProcessReceivedBuffer(TCPImpl & tcp, TCPEndPoint * endPoint, const PeerAddress & peerAddress, + System::PacketBufferHandle && buffer) + { + return tcp.ProcessReceivedBuffer(endPoint, peerAddress, std::move(buffer)); + } +}; +} // namespace Transport +} // namespace chip diff --git a/src/transport/raw/tests/TestMessageHeader.cpp b/src/transport/raw/tests/TestMessageHeader.cpp index 436bb887f4616e..b434d2b49138f0 100644 --- a/src/transport/raw/tests/TestMessageHeader.cpp +++ b/src/transport/raw/tests/TestMessageHeader.cpp @@ -23,14 +23,13 @@ * */ +#include #include #include #include #include #include -#include - namespace { using namespace chip; diff --git a/src/transport/raw/tests/TestPeerAddress.cpp b/src/transport/raw/tests/TestPeerAddress.cpp index 8d82e28ad58eb5..c2faffe55825cb 100644 --- a/src/transport/raw/tests/TestPeerAddress.cpp +++ b/src/transport/raw/tests/TestPeerAddress.cpp @@ -21,13 +21,12 @@ #include #include +#include #include #include #include #include -#include - namespace { using namespace chip; diff --git a/src/transport/raw/tests/TestTCP.cpp b/src/transport/raw/tests/TestTCP.cpp index 6a60fb330c70f7..24b35f0942199d 100644 --- a/src/transport/raw/tests/TestTCP.cpp +++ b/src/transport/raw/tests/TestTCP.cpp @@ -24,6 +24,7 @@ #include "NetworkTestHelpers.h" #include +#include #include #include #include @@ -34,8 +35,7 @@ #if INET_CONFIG_ENABLE_TCP_ENDPOINT #include #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT - -#include +#include #include #include @@ -43,6 +43,7 @@ #include using namespace chip; +using namespace chip::Test; using namespace chip::Inet; namespace { @@ -54,14 +55,13 @@ uint16_t gChipTCPPort = static_cast(CHIP_PORT chip::Transport::AppTCPConnectionCallbackCtxt gAppTCPConnCbCtxt; chip::Transport::ActiveTCPConnectionState * gActiveTCPConnState = nullptr; -using TCPImpl = Transport::TCP; +using TCPImpl = Transport::TCP; +using TestAccess = Transport::TCPBaseTestAccess; constexpr NodeId kSourceNodeId = 123654; constexpr NodeId kDestinationNodeId = 111222333; constexpr uint32_t kMessageCounter = 18; -using TestContext = chip::Test::IOContext; - const char PAYLOAD[] = "Hello!"; class MockTransportMgrDelegate : public chip::TransportMgrDelegate @@ -69,7 +69,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate public: typedef int (*MessageReceivedCallback)(const uint8_t * message, size_t length, int count, void * data); - MockTransportMgrDelegate(TestContext * inContext) : mContext(inContext), mCallback(nullptr), mCallbackData(nullptr) {} + MockTransportMgrDelegate(IOContext * inContext) : mIOContext(inContext), mCallback(nullptr), mCallbackData(nullptr) {} ~MockTransportMgrDelegate() override {} void SetCallback(MessageReceivedCallback callback = nullptr, void * callback_data = nullptr) @@ -134,7 +134,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate void InitializeMessageTest(TCPImpl & tcp, const IPAddress & addr) { - CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mContext->GetTCPEndPointManager()) + CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager()) .SetAddressType(addr.Type()) .SetListenPort(gChipTCPPort)); @@ -155,7 +155,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate { ChipLogProgress(NotSpecified, "RETRYING tcp initialization"); chip::test_utils::SleepMillis(100); - err = tcp.Init(Transport::TcpListenParameters(mContext->GetTCPEndPointManager()) + err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager()) .SetAddressType(addr.Type()) .SetListenPort(gChipTCPPort)); } @@ -193,7 +193,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate err = tcp.SendMessage(Transport::PeerAddress::TCP(addr, gChipTCPPort), std::move(buffer)); EXPECT_EQ(err, CHIP_NO_ERROR); - mContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [this]() { return mReceiveHandlerCallCount != 0; }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [this]() { return mReceiveHandlerCallCount != 0; }); EXPECT_EQ(mReceiveHandlerCallCount, 1); SetCallback(nullptr); @@ -205,7 +205,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate CHIP_ERROR err = tcp.TCPConnect(Transport::PeerAddress::TCP(addr, gChipTCPPort), &gAppTCPConnCbCtxt, &gActiveTCPConnState); EXPECT_EQ(err, CHIP_NO_ERROR); - mContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return tcp.HasActiveConnections(); }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return tcp.HasActiveConnections(); }); EXPECT_EQ(tcp.HasActiveConnections(), true); } @@ -216,7 +216,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate CHIP_ERROR err = tcp.TCPConnect(Transport::PeerAddress::TCP(addr, gChipTCPPort), &gAppTCPConnCbCtxt, &gActiveTCPConnState); EXPECT_EQ(err, CHIP_NO_ERROR); - mContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [this]() { return mHandleConnectionCompleteCalled; }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [this]() { return mHandleConnectionCompleteCalled; }); EXPECT_EQ(mHandleConnectionCompleteCalled, true); } @@ -227,11 +227,11 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate CHIP_ERROR err = tcp.TCPConnect(Transport::PeerAddress::TCP(addr, gChipTCPPort), &gAppTCPConnCbCtxt, &gActiveTCPConnState); EXPECT_EQ(err, CHIP_NO_ERROR); - mContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [this]() { return mHandleConnectionCompleteCalled; }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [this]() { return mHandleConnectionCompleteCalled; }); EXPECT_EQ(mHandleConnectionCompleteCalled, true); tcp.TCPDisconnect(Transport::PeerAddress::TCP(addr, gChipTCPPort)); - mContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return !tcp.HasActiveConnections(); }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return !tcp.HasActiveConnections(); }); EXPECT_EQ(mHandleConnectionCloseCalled, true); } @@ -239,7 +239,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate { // Disconnect and wait for seeing peer close tcp.TCPDisconnect(conn, true); - mContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return !tcp.HasActiveConnections(); }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return !tcp.HasActiveConnections(); }); EXPECT_EQ(tcp.HasActiveConnections(), false); } @@ -247,7 +247,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate { // Disconnect and wait for seeing peer close tcp.TCPDisconnect(Transport::PeerAddress::TCP(addr, gChipTCPPort)); - mContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return !tcp.HasActiveConnections(); }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(5), [&tcp]() { return !tcp.HasActiveConnections(); }); EXPECT_EQ(tcp.HasActiveConnections(), false); } @@ -280,7 +280,7 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate bool mHandleConnectionCloseCalled = false; private: - TestContext * mContext; + IOContext * mIOContext; MessageReceivedCallback mCallback; void * mCallbackData; TransportMgrBase mTransportMgrBase; @@ -289,153 +289,6 @@ class MockTransportMgrDelegate : public chip::TransportMgrDelegate OnTCPConnectionReceivedCallback mConnReceivedCb = nullptr; }; -class TestTCP : public ::testing::Test, public chip::Test::IOContext -{ -protected: - void SetUp() { ASSERT_EQ(Init(), CHIP_NO_ERROR); } - void TearDown() { Shutdown(); } - - /////////////////////////// Init test - void CheckSimpleInitTest(Inet::IPAddressType type) - { - TCPImpl tcp; - - CHIP_ERROR err = - tcp.Init(Transport::TcpListenParameters(GetTCPEndPointManager()).SetAddressType(type).SetListenPort(gChipTCPPort)); - - EXPECT_EQ(err, CHIP_NO_ERROR); - } - - /////////////////////////// Messaging test - void CheckMessageTest(const IPAddress & addr) - { - TCPImpl tcp; - - MockTransportMgrDelegate gMockTransportMgrDelegate(this); - gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); - gMockTransportMgrDelegate.SingleMessageTest(tcp, addr); - gMockTransportMgrDelegate.DisconnectTest(tcp, addr); - } - - void ConnectToSelfTest(const IPAddress & addr) - { - TCPImpl tcp; - - MockTransportMgrDelegate gMockTransportMgrDelegate(this); - gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); - gMockTransportMgrDelegate.ConnectTest(tcp, addr); - gMockTransportMgrDelegate.DisconnectTest(tcp, addr); - } - - void ConnectSendMessageThenCloseTest(const IPAddress & addr) - { - TCPImpl tcp; - - MockTransportMgrDelegate gMockTransportMgrDelegate(this); - gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); - gMockTransportMgrDelegate.ConnectTest(tcp, addr); - gMockTransportMgrDelegate.SingleMessageTest(tcp, addr); - gMockTransportMgrDelegate.DisconnectTest(tcp, addr); - } - - void HandleConnCompleteTest(const IPAddress & addr) - { - TCPImpl tcp; - - MockTransportMgrDelegate gMockTransportMgrDelegate(this); - gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); - gMockTransportMgrDelegate.HandleConnectCompleteCbCalledTest(tcp, addr); - gMockTransportMgrDelegate.DisconnectTest(tcp, addr); - } - - void HandleConnCloseTest(const IPAddress & addr) - { - TCPImpl tcp; - - MockTransportMgrDelegate gMockTransportMgrDelegate(this); - gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); - gMockTransportMgrDelegate.HandleConnectCloseCbCalledTest(tcp, addr); - gMockTransportMgrDelegate.DisconnectTest(tcp, addr); - } -}; - -#if INET_CONFIG_ENABLE_IPV4 -TEST_F(TestTCP, CheckSimpleInitTest4) -{ - CheckSimpleInitTest(IPAddressType::kIPv4); -} - -TEST_F(TestTCP, CheckMessageTest4) -{ - IPAddress addr; - IPAddress::FromString("127.0.0.1", addr); - CheckMessageTest(addr); -} -#endif - -TEST_F(TestTCP, CheckSimpleInitTest6) -{ - CheckSimpleInitTest(IPAddressType::kIPv6); -} - -TEST_F(TestTCP, CheckMessageTest6) -{ - IPAddress addr; - IPAddress::FromString("::1", addr); - CheckMessageTest(addr); -} - -#if INET_CONFIG_ENABLE_IPV4 -TEST_F(TestTCP, ConnectToSelfTest4) -{ - IPAddress addr; - IPAddress::FromString("127.0.0.1", addr); - ConnectToSelfTest(addr); -} - -TEST_F(TestTCP, ConnectSendMessageThenCloseTest4) -{ - IPAddress addr; - IPAddress::FromString("127.0.0.1", addr); - ConnectSendMessageThenCloseTest(addr); -} - -TEST_F(TestTCP, HandleConnCompleteCalledTest4) -{ - IPAddress addr; - IPAddress::FromString("127.0.0.1", addr); - HandleConnCompleteTest(addr); -} -#endif // INET_CONFIG_ENABLE_IPV4 - -TEST_F(TestTCP, ConnectToSelfTest6) -{ - IPAddress addr; - IPAddress::FromString("::1", addr); - ConnectToSelfTest(addr); -} - -TEST_F(TestTCP, ConnectSendMessageThenCloseTest6) -{ - IPAddress addr; - IPAddress::FromString("::1", addr); - ConnectSendMessageThenCloseTest(addr); -} - -TEST_F(TestTCP, HandleConnCompleteCalledTest6) -{ - IPAddress addr; - IPAddress::FromString("::1", addr); - HandleConnCompleteTest(addr); -} - -TEST_F(TestTCP, HandleConnCloseCalledTest6) -{ - IPAddress addr; - IPAddress::FromString("::1", addr); - HandleConnCloseTest(addr); -} - // Generates a packet buffer or a chain of packet buffers for a single message. struct TestData { @@ -563,108 +416,264 @@ void TestData::Free() mMessageOffset = 0; } -int TestDataCallbackCheck(const uint8_t * message, size_t length, int count, void * data) +class TestTCP : public ::testing::Test { - if (data == nullptr) +public: + static void SetUpTestSuite() { - return -1; + if (mIOContext == nullptr) + { + mIOContext = new IOContext(); + ASSERT_NE(mIOContext, nullptr); + } + ASSERT_EQ(mIOContext->Init(), CHIP_NO_ERROR); } - TestData * currentData = static_cast(data) + count; - if (currentData->mPayload == nullptr) + static void TearDownTestSuite() { - return -2; + if (mIOContext != nullptr) + { + mIOContext->Shutdown(); + delete mIOContext; + mIOContext = nullptr; + } } - if (currentData->mMessageLength != length) + +protected: + static IOContext * mIOContext; + + /////////////////////////// Init test + void CheckSimpleInitTest(IPAddressType type) { - return -3; + TCPImpl tcp; + + CHIP_ERROR err = tcp.Init( + Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager()).SetAddressType(type).SetListenPort(gChipTCPPort)); + + EXPECT_EQ(err, CHIP_NO_ERROR); } - if (memcmp(currentData->mPayload + currentData->mMessageOffset, message, length) != 0) + + /////////////////////////// Messaging test + void CheckMessageTest(const IPAddress & addr) { - return -4; - } - return 0; -} + TCPImpl tcp; -} // namespace + MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext); + gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); + gMockTransportMgrDelegate.SingleMessageTest(tcp, addr); + gMockTransportMgrDelegate.DisconnectTest(tcp, addr); + } -namespace chip { -namespace Transport { -class TCPTest -{ -public: - static void CheckProcessReceivedBuffer(TestContext * ctx) + void ConnectToSelfTest(const IPAddress & addr) { TCPImpl tcp; - IPAddress addr; - IPAddress::FromString("::1", addr); - - MockTransportMgrDelegate gMockTransportMgrDelegate(ctx); + MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext); gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); + gMockTransportMgrDelegate.ConnectTest(tcp, addr); + gMockTransportMgrDelegate.DisconnectTest(tcp, addr); + } - // Send a packet to get TCP going, so that we can find a TCPEndPoint to pass to ProcessReceivedBuffer. - // (The current TCPEndPoint implementation is not effectively mockable.) - gMockTransportMgrDelegate.SingleMessageTest(tcp, addr); + void ConnectSendMessageThenCloseTest(const IPAddress & addr) + { + TCPImpl tcp; - Transport::PeerAddress lPeerAddress = Transport::PeerAddress::TCP(addr, gChipTCPPort); - chip::Transport::ActiveTCPConnectionState * state = tcp.FindActiveConnection(lPeerAddress); - ASSERT_NE(state, nullptr); - Inet::TCPEndPoint * lEndPoint = state->mEndPoint; - ASSERT_NE(lEndPoint, nullptr); + MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext); + gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); + gMockTransportMgrDelegate.ConnectTest(tcp, addr); + gMockTransportMgrDelegate.SingleMessageTest(tcp, addr); + gMockTransportMgrDelegate.DisconnectTest(tcp, addr); + } - CHIP_ERROR err = CHIP_NO_ERROR; - TestData testData[2]; - gMockTransportMgrDelegate.SetCallback(TestDataCallbackCheck, testData); + void HandleConnCompleteTest(const IPAddress & addr) + { + TCPImpl tcp; - // Test a single packet buffer. - gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; - EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 111, 0 })); - err = tcp.ProcessReceivedBuffer(lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); - EXPECT_EQ(err, CHIP_NO_ERROR); - EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 1); + MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext); + gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); + gMockTransportMgrDelegate.HandleConnectCompleteCbCalledTest(tcp, addr); + gMockTransportMgrDelegate.DisconnectTest(tcp, addr); + } - // Test a message in a chain of three packet buffers. The message length is split across buffers. - gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; - EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 1, 122, 123, 0 })); - err = tcp.ProcessReceivedBuffer(lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); - EXPECT_EQ(err, CHIP_NO_ERROR); - EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 1); - - // Test two messages in a chain. - gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; - EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 131, 0 })); - EXPECT_TRUE(testData[1].Init((const uint16_t[]){ 132, 0 })); - testData[0].mHandle->AddToEnd(std::move(testData[1].mHandle)); - err = tcp.ProcessReceivedBuffer(lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); - EXPECT_EQ(err, CHIP_NO_ERROR); - EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 2); - - // Test a chain of two messages, each a chain. - gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; - EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 141, 142, 0 })); - EXPECT_TRUE(testData[1].Init((const uint16_t[]){ 143, 144, 0 })); - testData[0].mHandle->AddToEnd(std::move(testData[1].mHandle)); - err = tcp.ProcessReceivedBuffer(lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); - EXPECT_EQ(err, CHIP_NO_ERROR); - EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 2); - - // Test a message that is too large to coalesce into a single packet buffer. - gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; - gMockTransportMgrDelegate.SetCallback(TestDataCallbackCheck, &testData[1]); - EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 51, System::PacketBuffer::kMaxSizeWithoutReserve, 0 })); - // Sending only the first buffer of the long chain. This should be enough to trigger the error. - System::PacketBufferHandle head = testData[0].mHandle.PopHead(); - err = tcp.ProcessReceivedBuffer(lEndPoint, lPeerAddress, std::move(head)); - EXPECT_EQ(err, CHIP_ERROR_MESSAGE_TOO_LONG); - EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 0); + void HandleConnCloseTest(const IPAddress & addr) + { + TCPImpl tcp; + MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext); + gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); + gMockTransportMgrDelegate.HandleConnectCloseCbCalledTest(tcp, addr); gMockTransportMgrDelegate.DisconnectTest(tcp, addr); } + + // Callback used by CheckProcessReceivedBuffer. + static int TestDataCallbackCheck(const uint8_t * message, size_t length, int count, void * data) + { + if (data == nullptr) + { + return -1; + } + TestData * currentData = static_cast(data) + count; + if (currentData->mPayload == nullptr) + { + return -2; + } + if (currentData->mMessageLength != length) + { + return -3; + } + if (memcmp(currentData->mPayload + currentData->mMessageOffset, message, length) != 0) + { + return -4; + } + return 0; + } }; -} // namespace Transport -} // namespace chip + +IOContext * TestTCP::mIOContext = nullptr; + +#if INET_CONFIG_ENABLE_IPV4 +TEST_F(TestTCP, CheckSimpleInitTest4) +{ + CheckSimpleInitTest(IPAddressType::kIPv4); +} + +TEST_F(TestTCP, CheckMessageTest4) +{ + IPAddress addr; + IPAddress::FromString("127.0.0.1", addr); + CheckMessageTest(addr); +} +#endif + +TEST_F(TestTCP, CheckSimpleInitTest6) +{ + CheckSimpleInitTest(IPAddressType::kIPv6); +} + +TEST_F(TestTCP, CheckMessageTest6) +{ + IPAddress addr; + IPAddress::FromString("::1", addr); + CheckMessageTest(addr); +} + +#if INET_CONFIG_ENABLE_IPV4 +TEST_F(TestTCP, ConnectToSelfTest4) +{ + IPAddress addr; + IPAddress::FromString("127.0.0.1", addr); + ConnectToSelfTest(addr); +} + +TEST_F(TestTCP, ConnectSendMessageThenCloseTest4) +{ + IPAddress addr; + IPAddress::FromString("127.0.0.1", addr); + ConnectSendMessageThenCloseTest(addr); +} + +TEST_F(TestTCP, HandleConnCompleteCalledTest4) +{ + IPAddress addr; + IPAddress::FromString("127.0.0.1", addr); + HandleConnCompleteTest(addr); +} +#endif // INET_CONFIG_ENABLE_IPV4 + +TEST_F(TestTCP, ConnectToSelfTest6) +{ + IPAddress addr; + IPAddress::FromString("::1", addr); + ConnectToSelfTest(addr); +} + +TEST_F(TestTCP, ConnectSendMessageThenCloseTest6) +{ + IPAddress addr; + IPAddress::FromString("::1", addr); + ConnectSendMessageThenCloseTest(addr); +} + +TEST_F(TestTCP, HandleConnCompleteCalledTest6) +{ + IPAddress addr; + IPAddress::FromString("::1", addr); + HandleConnCompleteTest(addr); +} + +TEST_F(TestTCP, HandleConnCloseCalledTest6) +{ + IPAddress addr; + IPAddress::FromString("::1", addr); + HandleConnCloseTest(addr); +} TEST_F(TestTCP, CheckProcessReceivedBuffer) { - chip::Transport::TCPTest::CheckProcessReceivedBuffer(this); + TCPImpl tcp; + + IPAddress addr; + IPAddress::FromString("::1", addr); + + MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext); + gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); + + // Send a packet to get TCP going, so that we can find a TCPEndPoint to pass to ProcessReceivedBuffer. + // (The current TCPEndPoint implementation is not effectively mockable.) + gMockTransportMgrDelegate.SingleMessageTest(tcp, addr); + + Transport::PeerAddress lPeerAddress = Transport::PeerAddress::TCP(addr, gChipTCPPort); + void * state = TestAccess::FindActiveConnection(tcp, lPeerAddress); + ASSERT_NE(state, nullptr); + TCPEndPoint * lEndPoint = TestAccess::GetEndpoint(state); + ASSERT_NE(lEndPoint, nullptr); + + CHIP_ERROR err = CHIP_NO_ERROR; + TestData testData[2]; + gMockTransportMgrDelegate.SetCallback(TestDataCallbackCheck, testData); + + // Test a single packet buffer. + gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; + EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 111, 0 })); + err = TestAccess::ProcessReceivedBuffer(tcp, lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); + EXPECT_EQ(err, CHIP_NO_ERROR); + EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 1); + + // Test a message in a chain of three packet buffers. The message length is split across buffers. + gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; + EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 1, 122, 123, 0 })); + err = TestAccess::ProcessReceivedBuffer(tcp, lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); + EXPECT_EQ(err, CHIP_NO_ERROR); + EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 1); + + // Test two messages in a chain. + gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; + EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 131, 0 })); + EXPECT_TRUE(testData[1].Init((const uint16_t[]){ 132, 0 })); + testData[0].mHandle->AddToEnd(std::move(testData[1].mHandle)); + err = TestAccess::ProcessReceivedBuffer(tcp, lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); + EXPECT_EQ(err, CHIP_NO_ERROR); + EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 2); + + // Test a chain of two messages, each a chain. + gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; + EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 141, 142, 0 })); + EXPECT_TRUE(testData[1].Init((const uint16_t[]){ 143, 144, 0 })); + testData[0].mHandle->AddToEnd(std::move(testData[1].mHandle)); + err = TestAccess::ProcessReceivedBuffer(tcp, lEndPoint, lPeerAddress, std::move(testData[0].mHandle)); + EXPECT_EQ(err, CHIP_NO_ERROR); + EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 2); + + // Test a message that is too large to coalesce into a single packet buffer. + gMockTransportMgrDelegate.mReceiveHandlerCallCount = 0; + gMockTransportMgrDelegate.SetCallback(TestDataCallbackCheck, &testData[1]); + EXPECT_TRUE(testData[0].Init((const uint16_t[]){ 51, System::PacketBuffer::kMaxSizeWithoutReserve, 0 })); + // Sending only the first buffer of the long chain. This should be enough to trigger the error. + System::PacketBufferHandle head = testData[0].mHandle.PopHead(); + err = TestAccess::ProcessReceivedBuffer(tcp, lEndPoint, lPeerAddress, std::move(head)); + EXPECT_EQ(err, CHIP_ERROR_MESSAGE_TOO_LONG); + EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 0); + + gMockTransportMgrDelegate.DisconnectTest(tcp, addr); } + +} // namespace diff --git a/src/transport/raw/tests/TestUDP.cpp b/src/transport/raw/tests/TestUDP.cpp index 6a22a821fdfc94..5484711b9b9ddc 100644 --- a/src/transport/raw/tests/TestUDP.cpp +++ b/src/transport/raw/tests/TestUDP.cpp @@ -23,16 +23,16 @@ #include "NetworkTestHelpers.h" +#include #include #include #include #include -#include - #include using namespace chip; +using namespace chip::Test; using namespace chip::Inet; namespace { @@ -41,8 +41,6 @@ constexpr NodeId kSourceNodeId = 123654; constexpr NodeId kDestinationNodeId = 111222333; constexpr uint32_t kMessageCounter = 18; -using TestContext = chip::Test::IOContext; - const char PAYLOAD[] = "Hello!"; int ReceiveHandlerCallCount = 0; @@ -73,85 +71,106 @@ class MockTransportMgrDelegate : public TransportMgrDelegate } // namespace -/////////////////////////// Init test - class TestUDP : public ::testing::Test { +public: + // Performs shared setup for all tests in the test suite + static void SetUpTestSuite() + { + if (mIOContext == nullptr) + { + mIOContext = new IOContext(); + ASSERT_NE(mIOContext, nullptr); + } + ASSERT_EQ(mIOContext->Init(), CHIP_NO_ERROR); + } + + // Performs shared teardown for all tests in the test suite + static void TearDownTestSuite() + { + if (mIOContext != nullptr) + { + mIOContext->Shutdown(); + delete mIOContext; + mIOContext = nullptr; + } + } + protected: - TestUDP() { inContext = new TestContext(); } - ~TestUDP() { delete inContext; } - void SetUp() { ASSERT_EQ(inContext->Init(), CHIP_NO_ERROR); } - void TearDown() { inContext->Shutdown(); } - TestContext * inContext; -}; + static IOContext * mIOContext; -void CheckSimpleInitTest(TestContext * ctx, Inet::IPAddressType type) -{ - Transport::UDP udp; + void CheckSimpleInitTest(IPAddressType type) + { + Transport::UDP udp; - CHIP_ERROR err = udp.Init(Transport::UdpListenParameters(ctx->GetUDPEndPointManager()).SetAddressType(type).SetListenPort(0)); + CHIP_ERROR err = + udp.Init(Transport::UdpListenParameters(mIOContext->GetUDPEndPointManager()).SetAddressType(type).SetListenPort(0)); - EXPECT_EQ(err, CHIP_NO_ERROR); -} + EXPECT_EQ(err, CHIP_NO_ERROR); + } -void CheckMessageTest(TestContext * ctx, const IPAddress & addr) -{ - uint16_t payload_len = sizeof(PAYLOAD); + void CheckMessageTest(const IPAddress & addr) + { + uint16_t payload_len = sizeof(PAYLOAD); - chip::System::PacketBufferHandle buffer = chip::System::PacketBufferHandle::NewWithData(PAYLOAD, payload_len); - EXPECT_FALSE(buffer.IsNull()); + chip::System::PacketBufferHandle buffer = chip::System::PacketBufferHandle::NewWithData(PAYLOAD, payload_len); + EXPECT_FALSE(buffer.IsNull()); - CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; - Transport::UDP udp; + Transport::UDP udp; - err = udp.Init(Transport::UdpListenParameters(ctx->GetUDPEndPointManager()).SetAddressType(addr.Type()).SetListenPort(0)); - EXPECT_EQ(err, CHIP_NO_ERROR); + err = udp.Init( + Transport::UdpListenParameters(mIOContext->GetUDPEndPointManager()).SetAddressType(addr.Type()).SetListenPort(0)); + EXPECT_EQ(err, CHIP_NO_ERROR); - MockTransportMgrDelegate gMockTransportMgrDelegate; - TransportMgrBase gTransportMgrBase; - gTransportMgrBase.SetSessionManager(&gMockTransportMgrDelegate); - gTransportMgrBase.Init(&udp); + MockTransportMgrDelegate gMockTransportMgrDelegate; + TransportMgrBase gTransportMgrBase; + gTransportMgrBase.SetSessionManager(&gMockTransportMgrDelegate); + gTransportMgrBase.Init(&udp); - ReceiveHandlerCallCount = 0; + ReceiveHandlerCallCount = 0; - PacketHeader header; - header.SetSourceNodeId(kSourceNodeId).SetDestinationNodeId(kDestinationNodeId).SetMessageCounter(kMessageCounter); + PacketHeader header; + header.SetSourceNodeId(kSourceNodeId).SetDestinationNodeId(kDestinationNodeId).SetMessageCounter(kMessageCounter); - err = header.EncodeBeforeData(buffer); - EXPECT_EQ(err, CHIP_NO_ERROR); + err = header.EncodeBeforeData(buffer); + EXPECT_EQ(err, CHIP_NO_ERROR); - // Should be able to send a message to itself by just calling send. - err = udp.SendMessage(Transport::PeerAddress::UDP(addr, udp.GetBoundPort()), std::move(buffer)); - EXPECT_EQ(err, CHIP_NO_ERROR); + // Should be able to send a message to itself by just calling send. + err = udp.SendMessage(Transport::PeerAddress::UDP(addr, udp.GetBoundPort()), std::move(buffer)); + EXPECT_EQ(err, CHIP_NO_ERROR); - ctx->DriveIOUntil(chip::System::Clock::Seconds16(1), []() { return ReceiveHandlerCallCount != 0; }); + mIOContext->DriveIOUntil(chip::System::Clock::Seconds16(1), []() { return ReceiveHandlerCallCount != 0; }); - EXPECT_EQ(ReceiveHandlerCallCount, 1); -} + EXPECT_EQ(ReceiveHandlerCallCount, 1); + } +}; + +IOContext * TestUDP::mIOContext = nullptr; #if INET_CONFIG_ENABLE_IPV4 TEST_F(TestUDP, CheckSimpleInitTest4) { - CheckSimpleInitTest(inContext, IPAddressType::kIPv4); + CheckSimpleInitTest(IPAddressType::kIPv4); } TEST_F(TestUDP, CheckMessageTest4) { IPAddress addr; IPAddress::FromString("127.0.0.1", addr); - CheckMessageTest(inContext, addr); + CheckMessageTest(addr); } #endif TEST_F(TestUDP, CheckSimpleInitTest6) { - CheckSimpleInitTest(inContext, IPAddressType::kIPv6); + CheckSimpleInitTest(IPAddressType::kIPv6); } TEST_F(TestUDP, CheckMessageTest6) { IPAddress addr; IPAddress::FromString("::1", addr); - CheckMessageTest(inContext, addr); + CheckMessageTest(addr); } From e6549abccc05f33c7464eb9d0a0c5d21f808f7af Mon Sep 17 00:00:00 2001 From: Jeff Feasel Date: Sat, 4 May 2024 10:16:20 -0400 Subject: [PATCH 2/5] Clang formatting. --- src/transport/raw/TCP.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/transport/raw/TCP.h b/src/transport/raw/TCP.h index 5016157a08c985..783f3844d2d16d 100644 --- a/src/transport/raw/TCP.h +++ b/src/transport/raw/TCP.h @@ -42,7 +42,8 @@ namespace chip { namespace Transport { // Forward declaration of friend class for test access. -template class TCPBaseTestAccess; +template +class TCPBaseTestAccess; /** Defines listening parameters for setting up a TCP transport */ class TcpListenParameters @@ -204,7 +205,8 @@ class DLL_EXPORT TCPBase : public Base private: // Allow tests to access private members. - template friend class TCPBaseTestAccess; + template + friend class TCPBaseTestAccess; /** * Allocate an unused connection from the pool From dad7f87ec6cf70f2c1a3652ef110ce8a9b706547 Mon Sep 17 00:00:00 2001 From: Jeff Feasel Date: Mon, 6 May 2024 07:15:29 -0400 Subject: [PATCH 3/5] Added TCPBaseTestAccess.h to BUILD.gn --- src/transport/raw/tests/BUILD.gn | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/transport/raw/tests/BUILD.gn b/src/transport/raw/tests/BUILD.gn index 973626af7f340b..a6bc6a5a017524 100644 --- a/src/transport/raw/tests/BUILD.gn +++ b/src/transport/raw/tests/BUILD.gn @@ -49,6 +49,8 @@ chip_test_suite("tests") { test_sources += [ "TestTCP.cpp" ] } + sources = [ "TCPBaseTestAccess.h" ] + public_deps = [ ":helpers", "${chip_root}/src/inet/tests:helpers", From 1aa12e45018442bf8948dc28c77fcf735531f65c Mon Sep 17 00:00:00 2001 From: Jeff Feasel Date: Mon, 6 May 2024 07:44:54 -0400 Subject: [PATCH 4/5] Minor change to fix failing test --- src/transport/raw/tests/TCPBaseTestAccess.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/transport/raw/tests/TCPBaseTestAccess.h b/src/transport/raw/tests/TCPBaseTestAccess.h index 181583ecedd5f9..e89392f76c4daa 100644 --- a/src/transport/raw/tests/TCPBaseTestAccess.h +++ b/src/transport/raw/tests/TCPBaseTestAccess.h @@ -21,8 +21,6 @@ #include #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT -using namespace chip::Inet; - namespace chip { namespace Transport { /** @@ -39,9 +37,9 @@ class TCPBaseTestAccess { return tcp.FindActiveConnection(peerAddress); } - static TCPEndPoint * GetEndpoint(void * state) { return static_cast(state)->mEndPoint; } + static Inet::TCPEndPoint * GetEndpoint(void * state) { return static_cast(state)->mEndPoint; } - static CHIP_ERROR ProcessReceivedBuffer(TCPImpl & tcp, TCPEndPoint * endPoint, const PeerAddress & peerAddress, + static CHIP_ERROR ProcessReceivedBuffer(TCPImpl & tcp, Inet::TCPEndPoint * endPoint, const PeerAddress & peerAddress, System::PacketBufferHandle && buffer) { return tcp.ProcessReceivedBuffer(endPoint, peerAddress, std::move(buffer)); From d02d21c0271d84c52725fe1021054ae466f4c94d Mon Sep 17 00:00:00 2001 From: Jeff Feasel Date: Mon, 6 May 2024 10:23:31 -0400 Subject: [PATCH 5/5] Reordered the includes to match the standard "Related header, C system headers, C++ standard library headers, other libraries' headers, your project's headers" --- src/transport/raw/tests/TestMessageHeader.cpp | 1 + src/transport/raw/tests/TestPeerAddress.cpp | 1 + src/transport/raw/tests/TestTCP.cpp | 13 +++++++------ src/transport/raw/tests/TestUDP.cpp | 5 +++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/transport/raw/tests/TestMessageHeader.cpp b/src/transport/raw/tests/TestMessageHeader.cpp index b434d2b49138f0..4c807afc5c3608 100644 --- a/src/transport/raw/tests/TestMessageHeader.cpp +++ b/src/transport/raw/tests/TestMessageHeader.cpp @@ -24,6 +24,7 @@ */ #include + #include #include #include diff --git a/src/transport/raw/tests/TestPeerAddress.cpp b/src/transport/raw/tests/TestPeerAddress.cpp index c2faffe55825cb..d96bc1990e0755 100644 --- a/src/transport/raw/tests/TestPeerAddress.cpp +++ b/src/transport/raw/tests/TestPeerAddress.cpp @@ -22,6 +22,7 @@ #include #include + #include #include #include diff --git a/src/transport/raw/tests/TestTCP.cpp b/src/transport/raw/tests/TestTCP.cpp index 24b35f0942199d..f5e343f1a2ea79 100644 --- a/src/transport/raw/tests/TestTCP.cpp +++ b/src/transport/raw/tests/TestTCP.cpp @@ -23,8 +23,14 @@ #include "NetworkTestHelpers.h" -#include +#include +#include +#include +#include + #include + +#include #include #include #include @@ -37,11 +43,6 @@ #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT #include -#include -#include -#include -#include - using namespace chip; using namespace chip::Test; using namespace chip::Inet; diff --git a/src/transport/raw/tests/TestUDP.cpp b/src/transport/raw/tests/TestUDP.cpp index 5484711b9b9ddc..39167b89a58bb7 100644 --- a/src/transport/raw/tests/TestUDP.cpp +++ b/src/transport/raw/tests/TestUDP.cpp @@ -23,14 +23,15 @@ #include "NetworkTestHelpers.h" +#include + #include + #include #include #include #include -#include - using namespace chip; using namespace chip::Test; using namespace chip::Inet;