From 2b8d3a221e2fb00ca1d3573aa0d1060b90f331d1 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 9 Jul 2021 12:08:07 -0400 Subject: [PATCH] Add write roundtrip test (#8169) * Refactor TestWriteInteraction to use a MessagingContext. This allows us to test more details of the messages being sent. * Add a roundtrip test to TestWriteInteraction. Various fixups needed for that: 1) Make InteractionModelEngine::Shutdown actually remove the object as an unsolicited message handler. 2) Initialize WriteHandler's state properly so it's not random whether we think we have available WriteHandlers. 3) Fix TestWriteInteraction::AddAttributeDataElement to use the right tag (2, not 1) for its data. 4) Stop trying to run AppTests in QEMU, because of linking/compilation issues in that setup. --- scripts/tests/esp32_qemu_tests.sh | 16 ++- src/app/InteractionModelEngine.cpp | 2 + src/app/WriteHandler.h | 2 +- src/app/tests/BUILD.gn | 2 + src/app/tests/TestWriteInteraction.cpp | 154 ++++++++++++++++++------- 5 files changed, 128 insertions(+), 48 deletions(-) diff --git a/scripts/tests/esp32_qemu_tests.sh b/scripts/tests/esp32_qemu_tests.sh index da4bc967d05280..92bc1144027919 100755 --- a/scripts/tests/esp32_qemu_tests.sh +++ b/scripts/tests/esp32_qemu_tests.sh @@ -42,16 +42,16 @@ if [ $? -ne 0 ]; then fi really_run_suite() { - idf scripts/tools/qemu_run_test.sh src/test_driver/esp32/build/chip "$1" "$2" + idf scripts/tools/qemu_run_test.sh src/test_driver/esp32/build/chip "$@" } run_suite() { if [[ -d "${log_dir}" ]]; then suite=${1%.a} suite=${suite#lib} - really_run_suite "$1" "$2" |& tee "$log_dir/$suite.log" + really_run_suite "$@" |& tee "$log_dir/$suite.log" else - really_run_suite "$1" "$2" + really_run_suite "$@" fi } @@ -61,7 +61,15 @@ run_suite() { SUITES=( ) -run_suite libAppTests.a +# TODO: libAppTests depends on MessagingTestHelpers, which depends on +# NetworkTestHelpers. That sort of depends on InetTestHelpers or +# equivalent (to provide gSystemLayer, gInet, InitNetwork(), +# ShutdownNetwork()) but there's only a POSIX implementation of that +# last, which does not compile on ESP32. Need to figure out how to +# make that work. See comments below for the transport layer tests, +# which have the same issue. +# run_suite libAppTests.a -lMessagingTestHelpers -lNetworkTestHelpers + run_suite libASN1Tests.a run_suite libBleLayerTests.a run_suite libCoreTests.a diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 7697bfed89e4fd..73b87bba2a1af4 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -116,6 +116,8 @@ void InteractionModelEngine::Shutdown() } mpNextAvailableClusterInfo = nullptr; + + mpExchangeMgr->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id); } CHIP_ERROR InteractionModelEngine::NewCommandSender(CommandSender ** const apCommandSender) diff --git a/src/app/WriteHandler.h b/src/app/WriteHandler.h index cd731f94464b1b..2f4766582e6db7 100644 --- a/src/app/WriteHandler.h +++ b/src/app/WriteHandler.h @@ -106,7 +106,7 @@ class WriteHandler InteractionModelDelegate * mpDelegate = nullptr; WriteResponse::Builder mWriteResponseBuilder; System::PacketBufferTLVWriter mMessageWriter; - State mState; + State mState = State::Uninitialized; }; } // namespace app } // namespace chip diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index 738f4dd87d2dc1..53053889538fb6 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -41,7 +41,9 @@ chip_test_suite("tests") { "${chip_root}/src/app", "${chip_root}/src/app/util:device_callbacks_manager", "${chip_root}/src/lib/core", + "${chip_root}/src/messaging/tests:helpers", "${chip_root}/src/protocols", + "${chip_root}/src/transport/raw/tests:helpers", "${nlunit_test_root}:nlunit-test", ] } diff --git a/src/app/tests/TestWriteInteraction.cpp b/src/app/tests/TestWriteInteraction.cpp index 33967869fadc20..84fcfc533b5a18 100644 --- a/src/app/tests/TestWriteInteraction.cpp +++ b/src/app/tests/TestWriteInteraction.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -33,16 +34,17 @@ #include #include #include +#include #include namespace { -chip::System::Layer gSystemLayer; -chip::SecureSessionMgr gSessionManager; -chip::Messaging::ExchangeManager gExchangeManager; -chip::TransportMgr gTransportManager; -chip::secure_channel::MessageCounterManager gMessageCounterManager; -chip::Transport::AdminId gAdminId = 0; +chip::TransportMgrBase gTransportManager; +chip::Test::LoopbackTransport gLoopback; + +using TestContext = chip::Test::MessagingContext; +TestContext sContext; + } // namespace namespace chip { namespace app { @@ -51,6 +53,7 @@ class TestWriteInteraction public: static void TestWriteClient(nlTestSuite * apSuite, void * apContext); static void TestWriteHandler(nlTestSuite * apSuite, void * apContext); + static void TestWriteRoundtrip(nlTestSuite * apSuite, void * apContext); private: static void AddAttributeDataElement(nlTestSuite * apSuite, void * apContext, WriteClient & aWriteClient); @@ -86,7 +89,7 @@ void TestWriteInteraction::AddAttributeDataElement(nlTestSuite * apSuite, void * chip::TLV::TLVWriter * writer = aWriteClient.GetAttributeDataElementTLVWriter(); - err = writer->PutBoolean(chip::TLV::ContextTag(1), true); + err = writer->PutBoolean(chip::TLV::ContextTag(chip::app::AttributeDataElement::kCsTag_Data), true); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); err = aWriteClient.FinishAttribute(); @@ -198,18 +201,21 @@ void TestWriteInteraction::GenerateWriteResponse(nlTestSuite * apSuite, void * a void TestWriteInteraction::TestWriteClient(nlTestSuite * apSuite, void * apContext) { + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; app::WriteClient writeClient; chip::app::InteractionModelDelegate delegate; System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); - err = writeClient.Init(&gExchangeManager, &delegate); + err = writeClient.Init(&ctx.GetExchangeManager(), &delegate); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); AddAttributeDataElement(apSuite, apContext, writeClient); - err = writeClient.SendWriteRequest(kTestDeviceNodeId, gAdminId, nullptr); - NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_NOT_CONNECTED); + SecureSessionHandle session = ctx.GetSessionLocalToPeer(); + err = writeClient.SendWriteRequest(ctx.GetDestinationNodeId(), ctx.GetAdminId(), &session); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); GenerateWriteResponse(apSuite, apContext, buf); @@ -221,6 +227,8 @@ void TestWriteInteraction::TestWriteClient(nlTestSuite * apSuite, void * apConte void TestWriteInteraction::TestWriteHandler(nlTestSuite * apSuite, void * apContext) { + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; app::WriteHandler writeHandler; @@ -236,43 +244,72 @@ void TestWriteInteraction::TestWriteHandler(nlTestSuite * apSuite, void * apCont AddAttributeStatus(apSuite, apContext, writeHandler); - writeHandler.mpExchangeCtx = gExchangeManager.NewContext({ 0, 0, 0 }, nullptr); TestExchangeDelegate delegate; - writeHandler.mpExchangeCtx->SetDelegate(&delegate); - err = writeHandler.SendWriteResponse(); - NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_NOT_CONNECTED); + writeHandler.mpExchangeCtx = ctx.NewExchangeToLocal(&delegate); + err = writeHandler.SendWriteResponse(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); writeHandler.Shutdown(); } -} // namespace app -} // namespace chip -namespace { +CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & aReader, WriteHandler * aWriteHandler) +{ + return aWriteHandler->AddAttributeStatusCode( + AttributePathParams(aClusterInfo.mNodeId, aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mFieldId, + aClusterInfo.mListIndex, AttributePathParams::Flags::kFieldIdValid), + Protocols::SecureChannel::GeneralStatusCode::kSuccess, Protocols::SecureChannel::Id, + Protocols::InteractionModel::ProtocolCode::Success); +} -void InitializeChip(nlTestSuite * apSuite) +class RoundtripDelegate : public chip::app::InteractionModelDelegate { - CHIP_ERROR err = CHIP_NO_ERROR; - chip::Optional peer(chip::Transport::Type::kUndefined); - chip::Transport::AdminPairingTable admins; - chip::Transport::AdminPairingInfo * adminInfo = admins.AssignAdminId(gAdminId, chip::kTestDeviceNodeId); +public: + CHIP_ERROR WriteResponseStatus(const WriteClient * apWriteClient, + const Protocols::SecureChannel::GeneralStatusCode aGeneralCode, const uint32_t aProtocolId, + const uint16_t aProtocolCode, AttributePathParams & aAttributePathParams, + uint8_t aCommandIndex) override + { + mGotResponse = true; + return CHIP_NO_ERROR; + } - NL_TEST_ASSERT(apSuite, adminInfo != nullptr); + bool mGotResponse = false; +}; - err = chip::Platform::MemoryInit(); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); +void TestWriteInteraction::TestWriteRoundtrip(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); - gSystemLayer.Init(nullptr); + CHIP_ERROR err = CHIP_NO_ERROR; - err = gSessionManager.Init(chip::kTestDeviceNodeId, &gSystemLayer, &gTransportManager, &admins, &gMessageCounterManager); + RoundtripDelegate delegate; + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager(), &delegate); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = gExchangeManager.Init(&gSessionManager); + app::WriteClient * writeClient; + err = engine->NewWriteClient(&writeClient); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = gMessageCounterManager.Init(&gExchangeManager); + System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); + AddAttributeDataElement(apSuite, apContext, *writeClient); + + NL_TEST_ASSERT(apSuite, !delegate.mGotResponse); + + SecureSessionHandle session = ctx.GetSessionLocalToPeer(); + err = writeClient->SendWriteRequest(ctx.GetDestinationNodeId(), ctx.GetAdminId(), &session); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(apSuite, delegate.mGotResponse); + + engine->Shutdown(); } +} // namespace app +} // namespace chip + +namespace { + /** * Test Suite. It lists all the test functions. */ @@ -282,28 +319,59 @@ const nlTest sTests[] = { NL_TEST_DEF("CheckWriteClient", chip::app::TestWriteInteraction::TestWriteClient), NL_TEST_DEF("CheckWriteHandler", chip::app::TestWriteInteraction::TestWriteHandler), + NL_TEST_DEF("CheckWriteRoundtrip", chip::app::TestWriteInteraction::TestWriteRoundtrip), NL_TEST_SENTINEL() }; // clang-format on -} // namespace -int TestWriteInteraction() +int Initialize(void * aContext); +int Finalize(void * aContext); + +// clang-format off +nlTestSuite sSuite = +{ + "TestWriteInteraction", + &sTests[0], + Initialize, + Finalize +}; +// clang-format on + +int Initialize(void * aContext) +{ + CHIP_ERROR err = chip::Platform::MemoryInit(); + if (err != CHIP_NO_ERROR) + { + return FAILURE; + } + + gTransportManager.Init(&gLoopback); + + auto * ctx = static_cast(aContext); + err = ctx->Init(&sSuite, &gTransportManager); + if (err != CHIP_NO_ERROR) + { + return FAILURE; + } + + gTransportManager.SetSecureSessionMgr(&ctx->GetSecureSessionManager()); + return SUCCESS; +} + +int Finalize(void * aContext) { - // clang-format off - nlTestSuite theSuite = - { - "TestWriteInteraction", - &sTests[0], - nullptr, - nullptr - }; - // clang-format on + CHIP_ERROR err = reinterpret_cast(aContext)->Shutdown(); + chip::Platform::MemoryShutdown(); + return (err == CHIP_NO_ERROR) ? SUCCESS : FAILURE; +} - InitializeChip(&theSuite); +} // namespace - nlTestRunner(&theSuite, nullptr); +int TestWriteInteraction() +{ + nlTestRunner(&sSuite, &sContext); - return (nlTestRunnerStats(&theSuite)); + return (nlTestRunnerStats(&sSuite)); } CHIP_REGISTER_TEST_SUITE(TestWriteInteraction)