Use async loopback transport for all tests (#17622)

* Use async loopback transport for all tests

* Resolve comments

* Add comment about cleaning loopback queue

src/app/tests/AppTestContext.h

@@ -16,7 +16,6 @@
 #pragma once
 
 #include <messaging/tests/MessagingContext.h>
-#include <transport/raw/tests/NetworkTestHelpers.h>
 
 namespace chip {
 namespace Test {
@@ -25,14 +24,11 @@ namespace Test {
  * @brief The context of test cases for messaging layer. It wil initialize network layer and system layer, and create
  *        two secure sessions, connected with each other. Exchanges can be created for each secure session.
  */
-class AppContext : public LoopbackMessagingContext<>
+class AppContext : public LoopbackMessagingContext
 {
-    typedef LoopbackMessagingContext<> Super;
+    typedef LoopbackMessagingContext Super;
 
 public:
-    // Disallow initialization as a sync loopback context.
-    static void Initialize(void *) = delete;
-
     /// Initialize the underlying layers.
     CHIP_ERROR Init() override;
 

src/app/tests/TestBufferedReadCallback.cpp

@@ -609,7 +609,7 @@ nlTestSuite theSuite =
 {
     "TestBufferedReadCallback",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 

src/app/tests/TestClusterStateCache.cpp

@@ -625,7 +625,7 @@ nlTestSuite theSuite =
 {
     "TestClusterStateCache",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 

src/app/tests/TestCommandInteraction.cpp

@@ -924,7 +924,7 @@ nlTestSuite sSuite =
 {
     "TestCommandInteraction",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/app/tests/TestEventLogging.cpp

@@ -61,9 +61,9 @@ static chip::app::CircularEventBuffer gCircularEventBuffer[3];
 class TestContext : public chip::Test::AppContext
 {
 public:
-    static int InitializeAsync(void * context)
+    static int Initialize(void * context)
     {
-        if (AppContext::InitializeAsync(context) != SUCCESS)
+        if (AppContext::Initialize(context) != SUCCESS)
             return FAILURE;
 
         auto * ctx = static_cast<TestContext *>(context);
@@ -302,7 +302,7 @@ nlTestSuite sSuite =
 {
     "EventLogging",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/app/tests/TestEventOverflow.cpp

@@ -53,9 +53,9 @@ static chip::app::CircularEventBuffer gCircularEventBuffer[3];
 class TestContext : public chip::Test::AppContext
 {
 public:
-    static int InitializeAsync(void * context)
+    static int Initialize(void * context)
     {
-        if (AppContext::InitializeAsync(context) != SUCCESS)
+        if (AppContext::Initialize(context) != SUCCESS)
             return FAILURE;
 
         auto * ctx = static_cast<TestContext *>(context);
@@ -176,7 +176,7 @@ nlTestSuite sSuite =
 {
     "TestEventOverflow",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/app/tests/TestInteractionModelEngine.cpp

@@ -113,7 +113,7 @@ nlTestSuite sSuite =
 {
     "TestInteractionModelEngine",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/app/tests/TestReadInteraction.cpp

@@ -64,11 +64,9 @@ chip::DataVersion kTestDataVersion2     = 5;
 class TestContext : public chip::Test::AppContext
 {
 public:
-    static int Initialize(void * context) = delete;
-
-    static int InitializeAsync(void * context)
+    static int Initialize(void * context)
     {
-        if (AppContext::InitializeAsync(context) != SUCCESS)
+        if (AppContext::Initialize(context) != SUCCESS)
             return FAILURE;
 
         auto * ctx = static_cast<TestContext *>(context);
@@ -105,7 +103,6 @@ class TestContext : public chip::Test::AppContext
 };
 
 TestContext sContext;
-auto & sLoopback = sContext.GetLoopback();
 
 class TestEventGenerator : public chip::app::EventLoggingDelegate
 {
@@ -2609,7 +2606,7 @@ nlTestSuite sSuite =
 {
     "TestReadInteraction",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/app/tests/TestReportingEngine.cpp

@@ -195,7 +195,7 @@ nlTestSuite sSuite =
 {
     "TestReportingEngine",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/app/tests/TestTimedHandler.cpp

@@ -258,7 +258,7 @@ nlTestSuite sSuite =
 {
     "TestTimedHandler",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/app/tests/TestWriteInteraction.cpp

@@ -276,22 +276,6 @@ void TestWriteInteraction::TestWriteHandler(nlTestSuite * apSuite, void * apCont
 
     TestContext & ctx = *static_cast<TestContext *>(apContext);
 
-    //
-    // We have to enable async dispatch here to ensure that the exchange
-    // gets correctly closed out in the test below. Otherwise, the following happens:
-    //
-    // 1. WriteHandler generates a response upon OnWriteRequest being called.
-    // 2. Since there is no matching active client-side exchange for that request, the IM engine
-    //    handles it incorrectly and treats it like an unsolicited message.
-    // 3. It is invalid to receive a WriteResponse as an unsolicited message so it correctly sends back
-    //    a StatusResponse containing an error to that message.
-    // 4. Without unwinding the existing call stack, a response is received on the same exchange that the handler
-    //    generated a WriteResponse on. This exchange should have been closed in a normal execution model, but in
-    //    a synchronous model, the exchange is still open, and the status response is sent to the WriteHandler.
-    // 5. WriteHandler::OnMessageReceived is invoked, and it correctly asserts.
-    //
-    ctx.EnableAsyncDispatch();
-
     constexpr bool allBooleans[] = { true, false };
     for (auto messageIsTimed : allBooleans)
     {
@@ -328,15 +312,12 @@ void TestWriteInteraction::TestWriteHandler(nlTestSuite * apSuite, void * apCont
                 NL_TEST_ASSERT(apSuite, status == Status::UnsupportedAccess);
             }
 
-            ctx.DrainAndServiceIO();
             ctx.DrainAndServiceIO();
 
             Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr();
             NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);
         }
     }
-
-    ctx.DisableAsyncDispatch();
 }
 
 const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath)
@@ -490,7 +471,7 @@ int Test_Setup(void * inContext)
 {
     VerifyOrReturnError(CHIP_NO_ERROR == chip::Platform::MemoryInit(), FAILURE);
 
-    VerifyOrReturnError(TestContext::InitializeAsync(inContext) == SUCCESS, FAILURE);
+    VerifyOrReturnError(TestContext::Initialize(inContext) == SUCCESS, FAILURE);
 
     TestContext & ctx = *static_cast<TestContext *>(inContext);
     gTestStorage.ClearStorage();

src/controller/tests/TestEventCaching.cpp

@@ -54,9 +54,9 @@ static chip::app::CircularEventBuffer gCircularEventBuffer[3];
 class TestContext : public chip::Test::AppContext
 {
 public:
-    static int InitializeAsync(void * context)
+    static int Initialize(void * context)
     {
-        if (AppContext::InitializeAsync(context) != SUCCESS)
+        if (AppContext::Initialize(context) != SUCCESS)
             return FAILURE;
 
         auto * ctx = static_cast<TestContext *>(context);
@@ -454,7 +454,7 @@ nlTestSuite sSuite =
 {
     "TestEventCaching",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/controller/tests/TestEventChunking.cpp

@@ -54,9 +54,9 @@ static chip::app::CircularEventBuffer gCircularEventBuffer[3];
 class TestContext : public chip::Test::AppContext
 {
 public:
-    static int InitializeAsync(void * context)
+    static int Initialize(void * context)
     {
-        if (AppContext::InitializeAsync(context) != SUCCESS)
+        if (AppContext::Initialize(context) != SUCCESS)
             return FAILURE;
 
         auto * ctx = static_cast<TestContext *>(context);
@@ -534,7 +534,7 @@ nlTestSuite sSuite =
 {
     "TestEventChunking",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/controller/tests/TestReadChunking.cpp

@@ -908,7 +908,7 @@ nlTestSuite sSuite =
 {
     "TestReadChunking",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/controller/tests/TestServerCommandDispatch.cpp

@@ -184,8 +184,6 @@ void TestCommandInteraction::TestNoHandler(nlTestSuite * apSuite, void * apConte
 
     responseDirective = kSendDataResponse;
 
-    ctx.EnableAsyncDispatch();
-
     chip::Controller::InvokeCommandRequest(&ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, request, onSuccessCb,
                                            onFailureCb);
 
@@ -402,7 +400,7 @@ nlTestSuite sSuite =
 {
     "TestCommands",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/controller/tests/TestWriteChunking.cpp

@@ -727,7 +727,7 @@ nlTestSuite sSuite =
 {
     "TestWriteChunking",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/controller/tests/data_model/TestCommands.cpp

@@ -450,7 +450,7 @@ nlTestSuite sSuite =
 {
     "TestCommands",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/controller/tests/data_model/TestRead.cpp

@@ -1490,7 +1490,7 @@ nlTestSuite sSuite =
 {
     "TestRead",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/controller/tests/data_model/TestWrite.cpp

@@ -384,7 +384,7 @@ nlTestSuite sSuite =
 {
     "TestWrite",
     &sTests[0],
-    TestContext::InitializeAsync,
+    TestContext::Initialize,
     TestContext::Finalize
 };
 // clang-format on

src/messaging/ExchangeContext.cpp

@@ -424,25 +424,10 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
     // layer has completed its work on the ExchangeContext.
     ExchangeHandle ref(*this);
 
-    // Keep track of whether we're nested under an outer HandleMessage
-    // invocation.
-    bool alreadyHandlingMessage = mFlags.Has(Flags::kFlagHandlingMessage);
-    mFlags.Set(Flags::kFlagHandlingMessage);
-
     bool isStandaloneAck = payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::StandaloneAck);
     bool isDuplicate     = msgFlags.Has(MessageFlagValues::kDuplicateMessage);
 
     auto deferred = MakeDefer([&]() {
-        // The alreadyHandlingMessage check is effectively a workaround for the fact that SendMessage() is not calling
-        // MessageHandled() yet and will go away when we fix that.
-        if (alreadyHandlingMessage)
-        {
-            // Don't close if there's an outer HandleMessage invocation.  It'll deal with the closing.
-            return;
-        }
-        // We are the outermost HandleMessage invocation.  We're not handling a message anymore.
-        mFlags.Clear(Flags::kFlagHandlingMessage);
-
         // Duplicates and standalone acks are not application-level messages, so they should generally not lead to any state
         // changes.  The one exception to that is that if we have a null mDelegate then our lifetime is not application-defined,
         // since we don't interact with the application at that point.  That can happen when we are already closed (in which case

src/messaging/ReliableMessageContext.h

@@ -198,14 +198,11 @@ class ReliableMessageContext
         /// When set, signifies that this exchange is waiting for a call to SendMessage.
         kFlagWillSendMessage = (1u << 8),
 
-        /// When set, signifies that we are currently in the middle of HandleMessage.
-        kFlagHandlingMessage = (1u << 9),
-
         /// When set, we have had Close() or Abort() called on us already.
-        kFlagClosed = (1u << 10),
+        kFlagClosed = (1u << 9),
 
         /// When set, signifies that the exchange is requesting Sleepy End Device fast-polling mode.
-        kFlagFastPollingMode = (1u << 11),
+        kFlagFastPollingMode = (1u << 10),
     };
 
     BitFlags<Flags> mFlags; // Internal state flags

src/messaging/tests/BUILD.gn

@@ -35,7 +35,7 @@ static_library("helpers") {
     "${chip_root}/src/messaging",
     "${chip_root}/src/protocols",
     "${chip_root}/src/transport",
-    "${chip_root}/src/transport/raw/tests:helpers",
+    "${chip_root}/src/transport/tests:helpers",
     "${nlio_root}:nlio",
   ]
 }