From b951c6cb09a9eb043c8480b0a493fb0794697590 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 28 Feb 2024 13:14:43 -0500 Subject: [PATCH] Cleanup and fix batch command tests in TestCommandInteraction (#32288) * Have unit test build batch command using CommandSender * Restyled by clang-format * Fix CI * Restyled by clang-format --------- Co-authored-by: Restyled.io --- src/app/tests/TestCommandInteraction.cpp | 201 +++++++++++------------ 1 file changed, 96 insertions(+), 105 deletions(-) diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index 02bf686248c1df..d917315f2a43ec 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -1576,36 +1576,31 @@ void TestCommandInteraction::TestCommandHandlerRejectMultipleIdenticalCommands(n CHIP_ERROR err = CHIP_NO_ERROR; isCommandDispatched = false; - mockCommandSenderDelegate.ResetCounter(); - app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + mockCommandSenderExtendedDelegate.ResetCounter(); + PendingResponseTrackerImpl pendingResponseTracker; + app::CommandSender commandSender(kCommandSenderTestOnlyMarker, &mockCommandSenderExtendedDelegate, &ctx.GetExchangeManager(), + &pendingResponseTracker); - { - // Command ID is not important here, since the command handler should reject the commands without handling it. - auto commandPathParams = MakeTestCommandPath(kTestCommandIdCommandSpecificResponse); + app::CommandSender::ConfigParameters configParameters; + configParameters.SetRemoteMaxPathsPerInvoke(2); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.SetCommandSenderConfig(configParameters)); - commandSender.AllocateBuffer(); + // Command ID is not important here, since the command handler should reject the commands without handling it. + auto commandPathParams = MakeTestCommandPath(kTestCommandIdCommandSpecificResponse); - // TODO(#30453): CommandSender does support sending multiple commands, update this test to use that. - for (int i = 0; i < 2; i++) - { - InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); - CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); - CommandPathIB::Builder & path = invokeRequest.CreatePath(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(commandPathParams)); - NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == - invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), - TLV::kTLVType_Structure, - commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); - NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB()); - } + for (uint16_t i = 0; i < 2; i++) + { + app::CommandSender::PrepareCommandParameters prepareCommandParams; + prepareCommandParams.SetStartDataStruct(true); + prepareCommandParams.SetCommandRef(i); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.PrepareCommand(commandPathParams, prepareCommandParams)); + NL_TEST_ASSERT(apSuite, + CHIP_NO_ERROR == commandSender.GetCommandDataIBTLVWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); - commandSender.MoveToState(app::CommandSender::State::AddedCommand); + app::CommandSender::FinishCommandParameters finishCommandParams; + finishCommandParams.SetEndDataStruct(true); + finishCommandParams.SetCommandRef(i); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.FinishCommand(finishCommandParams)); } err = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice()); @@ -1615,14 +1610,17 @@ void TestCommandInteraction::TestCommandHandlerRejectMultipleIdenticalCommands(n ctx.DrainAndServiceIO(); NL_TEST_ASSERT(apSuite, - mockCommandSenderDelegate.onResponseCalledTimes == 0 && mockCommandSenderDelegate.onFinalCalledTimes == 1 && - mockCommandSenderDelegate.onErrorCalledTimes == 1); + mockCommandSenderExtendedDelegate.onResponseCalledTimes == 0 && + mockCommandSenderExtendedDelegate.onFinalCalledTimes == 1 && + mockCommandSenderExtendedDelegate.onErrorCalledTimes == 1); NL_TEST_ASSERT(apSuite, !chip::isCommandDispatched); NL_TEST_ASSERT(apSuite, GetNumActiveHandlerObjects() == 0); NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + void TestCommandInteraction::TestCommandHandlerRejectsMultipleCommandsWithIdenticalCommandRef(nlTestSuite * apSuite, void * apContext) { @@ -1630,45 +1628,44 @@ void TestCommandInteraction::TestCommandHandlerRejectsMultipleCommandsWithIdenti CHIP_ERROR err = CHIP_NO_ERROR; isCommandDispatched = false; - mockCommandSenderDelegate.ResetCounter(); + mockCommandSenderExtendedDelegate.ResetCounter(); + PendingResponseTrackerImpl pendingResponseTracker; + app::CommandSender commandSender(kCommandSenderTestOnlyMarker, &mockCommandSenderExtendedDelegate, &ctx.GetExchangeManager(), + &pendingResponseTracker); - // Using commandSender to help build afterward we take the buffer to feed into standalone CommandHandler - app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + app::CommandSender::ConfigParameters configParameters; + configParameters.SetRemoteMaxPathsPerInvoke(2); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.SetCommandSenderConfig(configParameters)); - size_t numberOfCommandsToSend = 2; + uint16_t numberOfCommandsToSend = 2; { CommandPathParams requestCommandPaths[] = { MakeTestCommandPath(kTestCommandIdWithData), MakeTestCommandPath(kTestCommandIdCommandSpecificResponse), }; - commandSender.AllocateBuffer(); + uint16_t hardcodedCommandRef = 0; - // TODO(#30453): CommandSender does support sending multiple commands, update this test to use that. - for (size_t i = 0; i < numberOfCommandsToSend; i++) + for (uint16_t i = 0; i < numberOfCommandsToSend; i++) { - InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); - CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); - CommandPathIB::Builder & path = invokeRequest.CreatePath(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(requestCommandPaths[i])); + app::CommandSender::PrepareCommandParameters prepareCommandParams; + prepareCommandParams.SetStartDataStruct(true); + prepareCommandParams.SetCommandRef(i); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.PrepareCommand(requestCommandPaths[i], prepareCommandParams)); NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == - invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), - TLV::kTLVType_Structure, - commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); - NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.Ref(1)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB()); + CHIP_NO_ERROR == commandSender.GetCommandDataIBTLVWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + // TODO fix this comment + // We are taking advantage of the fact that the commandRef was set into finishCommandParams during PrepareCommand + // But setting it to a different value here, we are overriding what it was supposed to be. + app::CommandSender::FinishCommandParameters finishCommandParams; + finishCommandParams.SetEndDataStruct(true); + finishCommandParams.SetCommandRef(hardcodedCommandRef); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.TestOnlyFinishCommand(finishCommandParams)); } - - commandSender.MoveToState(app::CommandSender::State::AddedCommand); } - CommandHandler commandHandler(&mockCommandHandlerDelegate); + BasicCommandPathRegistry<4> mBasicCommandPathRegistry; + CommandHandler commandHandler(kCommandHandlerTestOnlyMarker, &mockCommandHandlerDelegate, &mBasicCommandPathRegistry); TestExchangeDelegate delegate; auto exchange = ctx.NewExchangeToAlice(&delegate, false); commandHandler.mResponseSender.SetExchangeContext(exchange); @@ -1696,6 +1693,8 @@ void TestCommandInteraction::TestCommandHandlerRejectsMultipleCommandsWithIdenti exchange->Close(); } +#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST + void TestCommandInteraction::TestCommandHandlerRejectMultipleCommandsWhenHandlerOnlySupportsOne(nlTestSuite * apSuite, void * apContext) { @@ -1703,46 +1702,39 @@ void TestCommandInteraction::TestCommandHandlerRejectMultipleCommandsWhenHandler CHIP_ERROR err = CHIP_NO_ERROR; isCommandDispatched = false; - mockCommandSenderDelegate.ResetCounter(); - // Using commandSender to help build afterward we take the buffer to feed into standalone CommandHandler - app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + mockCommandSenderExtendedDelegate.ResetCounter(); + PendingResponseTrackerImpl pendingResponseTracker; + app::CommandSender commandSender(kCommandSenderTestOnlyMarker, &mockCommandSenderExtendedDelegate, &ctx.GetExchangeManager(), + &pendingResponseTracker); - size_t numberOfCommandsToSend = 2; + app::CommandSender::ConfigParameters configParameters; + configParameters.SetRemoteMaxPathsPerInvoke(2); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.SetCommandSenderConfig(configParameters)); + + uint16_t numberOfCommandsToSend = 2; { CommandPathParams requestCommandPaths[] = { MakeTestCommandPath(kTestCommandIdWithData), MakeTestCommandPath(kTestCommandIdCommandSpecificResponse), }; - commandSender.AllocateBuffer(); - - // TODO(#30453): CommandSender does support sending multiple commands, update this test to use that. - for (size_t i = 0; i < numberOfCommandsToSend; i++) + for (uint16_t i = 0; i < numberOfCommandsToSend; i++) { - InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); - CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); - CommandPathIB::Builder & path = invokeRequest.CreatePath(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(requestCommandPaths[i])); + app::CommandSender::PrepareCommandParameters prepareCommandParams; + prepareCommandParams.SetStartDataStruct(true); + prepareCommandParams.SetCommandRef(i); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.PrepareCommand(requestCommandPaths[i], prepareCommandParams)); NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == - invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), - TLV::kTLVType_Structure, - commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); - NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.Ref(static_cast(i))); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB()); + CHIP_NO_ERROR == commandSender.GetCommandDataIBTLVWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + app::CommandSender::FinishCommandParameters finishCommandParams; + finishCommandParams.SetEndDataStruct(true); + finishCommandParams.SetCommandRef(i); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.FinishCommand(finishCommandParams)); } - - commandSender.MoveToState(app::CommandSender::State::AddedCommand); } - BasicCommandPathRegistry<4> mBasicCommandPathRegistry; - CommandHandler commandHandler(kCommandHandlerTestOnlyMarker, &mockCommandHandlerDelegate, &mBasicCommandPathRegistry); + CommandHandler commandHandler(&mockCommandHandlerDelegate); TestExchangeDelegate delegate; auto exchange = ctx.NewExchangeToAlice(&delegate, false); commandHandler.mResponseSender.SetExchangeContext(exchange); @@ -1757,9 +1749,9 @@ void TestCommandInteraction::TestCommandHandlerRejectMultipleCommandsWhenHandler commandDispatchedCount = 0; InteractionModel::Status status = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), false); - NL_TEST_ASSERT(apSuite, status == InteractionModel::Status::Success); + NL_TEST_ASSERT(apSuite, status == InteractionModel::Status::InvalidAction); - NL_TEST_ASSERT(apSuite, commandDispatchedCount == 2); + NL_TEST_ASSERT(apSuite, commandDispatchedCount == 0); // // Ordinarily, the ExchangeContext will close itself on a responder exchange when unwinding back from an @@ -1777,39 +1769,35 @@ void TestCommandInteraction::TestCommandHandlerAcceptMultipleCommands(nlTestSuit CHIP_ERROR err = CHIP_NO_ERROR; isCommandDispatched = false; - mockCommandSenderDelegate.ResetCounter(); - // Using commandSender to help build afterward we take the buffer to feed into standalone CommandHandler - app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager()); + mockCommandSenderExtendedDelegate.ResetCounter(); + PendingResponseTrackerImpl pendingResponseTracker; + app::CommandSender commandSender(kCommandSenderTestOnlyMarker, &mockCommandSenderExtendedDelegate, &ctx.GetExchangeManager(), + &pendingResponseTracker); + + app::CommandSender::ConfigParameters configParameters; + configParameters.SetRemoteMaxPathsPerInvoke(2); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.SetCommandSenderConfig(configParameters)); - size_t numberOfCommandsToSend = 2; + uint16_t numberOfCommandsToSend = 2; { CommandPathParams requestCommandPaths[] = { MakeTestCommandPath(kTestCommandIdWithData), MakeTestCommandPath(kTestCommandIdCommandSpecificResponse), }; - commandSender.AllocateBuffer(); - - // TODO(#30453): CommandSender does support sending multiple commands, update this test to use that. - for (size_t i = 0; i < numberOfCommandsToSend; i++) + for (uint16_t i = 0; i < numberOfCommandsToSend; i++) { - InvokeRequests::Builder & invokeRequests = commandSender.mInvokeRequestBuilder.GetInvokeRequests(); - CommandDataIB::Builder & invokeRequest = invokeRequests.CreateCommandData(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequests.GetError()); - CommandPathIB::Builder & path = invokeRequest.CreatePath(); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetError()); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == path.Encode(requestCommandPaths[i])); - NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == - invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(CommandDataIB::Tag::kFields), - TLV::kTLVType_Structure, - commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.GetWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + app::CommandSender::PrepareCommandParameters prepareCommandParams; + prepareCommandParams.SetStartDataStruct(true); + prepareCommandParams.SetCommandRef(i); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.PrepareCommand(requestCommandPaths[i], prepareCommandParams)); NL_TEST_ASSERT(apSuite, - CHIP_NO_ERROR == invokeRequest.GetWriter()->EndContainer(commandSender.mDataElementContainerType)); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.Ref(static_cast(i))); - NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == invokeRequest.EndOfCommandDataIB()); + CHIP_NO_ERROR == commandSender.GetCommandDataIBTLVWriter()->PutBoolean(chip::TLV::ContextTag(1), true)); + app::CommandSender::FinishCommandParameters finishCommandParams; + finishCommandParams.SetEndDataStruct(true); + finishCommandParams.SetCommandRef(i); + NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == commandSender.FinishCommand(finishCommandParams)); } commandSender.MoveToState(app::CommandSender::State::AddedCommand); @@ -1826,6 +1814,7 @@ void TestCommandInteraction::TestCommandHandlerAcceptMultipleCommands(nlTestSuit err = commandSender.Finalize(commandDatabuf); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + sendResponse = true; mockCommandHandlerDelegate.ResetCounter(); commandDispatchedCount = 0; @@ -1980,7 +1969,9 @@ const nlTest sTests[] = NL_TEST_DEF("TestCommandHandlerWithProcessReceivedNotExistCommand", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistCommand), NL_TEST_DEF("TestCommandHandlerWithProcessReceivedEmptyDataMsg", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg), NL_TEST_DEF("TestCommandHandlerRejectMultipleIdenticalCommands", chip::app::TestCommandInteraction::TestCommandHandlerRejectMultipleIdenticalCommands), +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST NL_TEST_DEF("TestCommandHandlerRejectsMultipleCommandsWithIdenticalCommandRef", chip::app::TestCommandInteraction::TestCommandHandlerRejectsMultipleCommandsWithIdenticalCommandRef), +#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST NL_TEST_DEF("TestCommandHandlerRejectMultipleCommandsWhenHandlerOnlySupportsOne", chip::app::TestCommandInteraction::TestCommandHandlerRejectMultipleCommandsWhenHandlerOnlySupportsOne), NL_TEST_DEF("TestCommandHandlerAcceptMultipleCommands", chip::app::TestCommandInteraction::TestCommandHandlerAcceptMultipleCommands), NL_TEST_DEF("TestCommandHandler_FillUpInvokeResponseMessageWhereSecondResponseIsStatusResponse", chip::app::TestCommandInteraction::TestCommandHandler_FillUpInvokeResponseMessageWhereSecondResponseIsStatusResponse),