From 4dd9914313ad00458bbadac0f95985fe717fa3ee Mon Sep 17 00:00:00 2001 From: Yunhan Wang Date: Sat, 2 Jul 2022 00:58:38 -0700 Subject: [PATCH] address comments --- src/app/tests/TestCommandInteraction.cpp | 9 +++++++- src/app/tests/TestReadInteraction.cpp | 21 +++++++------------ .../tests/LoopbackTransportManager.h | 19 ++++++++++++++++- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index 3d089a6b2eadde..22c5d911008320 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -662,9 +662,10 @@ void TestCommandInteraction::TestCommandInvalidMessage1(nlTestSuite * apSuite, v AddInvokeRequestData(apSuite, apContext, &commandSender); asyncCommand = true; err = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - ctx.DeliverOneMessage(); + ctx.DrainAndServiceIO(); NL_TEST_ASSERT(apSuite, mockCommandSenderDelegate.onResponseCalledTimes == 0 && mockCommandSenderDelegate.onFinalCalledTimes == 0 && @@ -672,6 +673,7 @@ void TestCommandInteraction::TestCommandInvalidMessage1(nlTestSuite * apSuite, v NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 1); NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 2); + commandSender.MoveToState(app::CommandSender::State::ResponseReceived); System::PacketBufferHandle msgBuf = System::PacketBufferHandle::New(kMaxSecureSduLengthBytes); @@ -696,9 +698,14 @@ void TestCommandInteraction::TestCommandInvalidMessage1(nlTestSuite * apSuite, v // Decrease CommandHandler refcount and send response asyncCommandHandle = nullptr; + ctx.GetLoopback().mSentMessageCount = 0; ctx.DrainAndServiceIO(); + // a command response message was sent + printf("debug %d", ctx.GetLoopback().mSentMessageCount); + NL_TEST_ASSERT(apSuite, ctx.GetLoopback().mSentMessageCount == 1); NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index b3f278b1cfed7f..91f66b218f80a3 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -2741,7 +2741,6 @@ void TestReadInteraction::TestReadInvalidMessage1(nlTestSuite * apSuite, void * auto * engine = chip::app::InteractionModelEngine::GetInstance(); err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); @@ -2761,7 +2760,7 @@ void TestReadInteraction::TestReadInvalidMessage1(nlTestSuite * apSuite, void * err = readClient.mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf), Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - ctx.DeliverOneMessage(); + ctx.DrainAndServiceIO(); NL_TEST_ASSERT(apSuite, delegate.mError != CHIP_NO_ERROR); } NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); @@ -2783,7 +2782,6 @@ void TestReadInteraction::TestReadInvalidMessage2(nlTestSuite * apSuite, void * auto * engine = chip::app::InteractionModelEngine::GetInstance(); err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); @@ -2831,7 +2829,7 @@ void TestReadInteraction::TestReadInvalidMessage2(nlTestSuite * apSuite, void * } // Read Client creates the subscription with server, server sends chunked reports, after the hander sends out the first chunked report, -// handle call unknow message function and send status report, client +// handler calls unknown message function and send status report, client // and server would be closed void TestReadInteraction::TestSubscribeInvalidMessage1(nlTestSuite * apSuite, void * apContext) { @@ -2842,13 +2840,10 @@ void TestReadInteraction::TestSubscribeInvalidMessage1(nlTestSuite * apSuite, vo // Shouldn't have anything in the retransmit table when starting the test. NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); - GenerateEvents(apSuite, apContext); - MockInteractionModelApp delegate; auto * engine = chip::app::InteractionModelEngine::GetInstance(); err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); chip::app::AttributePathParams attributePathParams[1]; // Mock Attribute 4 is a big attribute, with 6 large OCTET_STRING @@ -2857,8 +2852,6 @@ void TestReadInteraction::TestSubscribeInvalidMessage1(nlTestSuite * apSuite, vo attributePathParams[0].mAttributeId = Test::MockAttributeId(4); ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); - readPrepareParams.mpEventPathParamsList = nullptr; - readPrepareParams.mEventPathParamsListSize = 0; readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = 1; @@ -2871,12 +2864,14 @@ void TestReadInteraction::TestSubscribeInvalidMessage1(nlTestSuite * apSuite, vo ctx.DeliverOneMessage(); + //ctx.GetLoopback().mNumMessagesToDrop = 1; + //ctx.DeliverOneMessage(); + System::PacketBufferHandle msgBuf; ReadRequestMessage::Builder request; System::PacketBufferTLVWriter writer; /* - //TODO: Send real unknown message to trigger unknown message flow in handler chip::app::InitWriterWithSpaceReserved(writer, 0); err = request.Init(&writer); err = writer.Finalize(&msgBuf); @@ -2914,7 +2909,6 @@ void TestReadInteraction::TestSubscribeInvalidMessage2(nlTestSuite * apSuite, vo auto * engine = chip::app::InteractionModelEngine::GetInstance(); err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); @@ -2934,7 +2928,7 @@ void TestReadInteraction::TestSubscribeInvalidMessage2(nlTestSuite * apSuite, vo err = readClient.mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeRequest, std::move(msgBuf), Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - ctx.DeliverOneMessage(); + ctx.DrainAndServiceIO(); NL_TEST_ASSERT(apSuite, delegate.mError != CHIP_NO_ERROR); } NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); @@ -2956,7 +2950,6 @@ void TestReadInteraction::TestSubscribeInvalidMessage3(nlTestSuite * apSuite, vo auto * engine = chip::app::InteractionModelEngine::GetInstance(); err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); @@ -3041,7 +3034,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestSubscribeRoundtrip", chip::app::TestReadInteraction::TestSubscribeRoundtrip), NL_TEST_DEF("TestReadInvalidMessage1", chip::app::TestReadInteraction::TestReadInvalidMessage1), NL_TEST_DEF("TestReadInvalidMessage2", chip::app::TestReadInteraction::TestReadInvalidMessage2), - //NL_TEST_DEF("TestSubscribeInvalidMessage1", chip::app::TestReadInteraction::TestSubscribeInvalidMessage1), + NL_TEST_DEF("TestSubscribeInvalidMessage1", chip::app::TestReadInteraction::TestSubscribeInvalidMessage1), NL_TEST_DEF("TestSubscribeInvalidMessage2", chip::app::TestReadInteraction::TestSubscribeInvalidMessage2), NL_TEST_DEF("TestSubscribeInvalidMessage3", chip::app::TestReadInteraction::TestSubscribeInvalidMessage3), NL_TEST_DEF("TestSubscribeUrgentWildcardEvent", chip::app::TestReadInteraction::TestSubscribeUrgentWildcardEvent), diff --git a/src/transport/tests/LoopbackTransportManager.h b/src/transport/tests/LoopbackTransportManager.h index 286d7b7a7f6fe7..687c8307dabf96 100644 --- a/src/transport/tests/LoopbackTransportManager.h +++ b/src/transport/tests/LoopbackTransportManager.h @@ -97,7 +97,24 @@ class LoopbackTransportManager } } - void DeliverOneMessage() { mIOContext.DriveIO(); } + /* + * This drives the servicing of events using the embedded IOContext while there are pending + * messages in the loopback transport's pending message queue. This should run to completion for one message + */ + void DeliverOneMessage(System::Clock::Timeout maxWait = chip::System::Clock::Seconds16(5)) + { + auto & impl = GetLoopback(); + System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + if (!impl.HasPendingMessages()) + { + return; + } + mIOContext.DriveIO(); + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= maxWait) + { + return; + } + } private: Test::IOContext mIOContext;