Fix failing TestTimedHandler unit test. (#12470)
This is failing due to a merge conflict between
https://github.com/project-chip/connectedhomeip/pull/11988 and
https://github.com/project-chip/connectedhomeip/pull/12389: the latter
ends up in an error state as described in
https://github.com/project-chip/connectedhomeip/pull/12466#issue-793500688
and the former makes our code a lot more sensitive to being in that
error state.
The fix for the test is to not use the sync mode of loopback
transport, which allows the stack for sending a message to unwind
before responses are delivered and avoids the "object deleted by
response while we are still working with it" problem described in
https://github.com/project-chip/connectedhomeip/pull/12466#issue-793500688.
When the responses were made async, it turned out the test was missing
some "expect response" flags that should have been there all along and
it was only passing because the response happened before the send
could get to the "close the exchange" stage. With async responses,
exchanges were closing too early without the "expect response" flags.
Separately we should figure out which parts of
https://github.com/project-chip/connectedhomeip/pull/12466 we should
do.
diff --git a/src/app/tests/TestTimedHandler.cpp b/src/app/tests/TestTimedHandler.cpp
index cf6dfd9..63d7f19 100644
--- a/src/app/tests/TestTimedHandler.cpp
+++ b/src/app/tests/TestTimedHandler.cpp
@@ -124,6 +124,8 @@
CHIP_ERROR err = exchange->SendMessage(MsgType::TimedRequest, std::move(payload), SendMessageFlags::kExpectResponse);
NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
NL_TEST_ASSERT(aSuite, delegate.mNewMessageReceived);
NL_TEST_ASSERT(aSuite, delegate.mLastMessageWasStatus);
NL_TEST_ASSERT(aSuite, delegate.mStatus.mStatus == Status::Success);
@@ -136,8 +138,10 @@
delegate.mKeepExchangeOpen = false;
delegate.mNewMessageReceived = false;
- err = exchange->SendMessage(aMsgType, std::move(payload));
+ err = exchange->SendMessage(aMsgType, std::move(payload), SendMessageFlags::kExpectResponse);
NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
NL_TEST_ASSERT(aSuite, delegate.mNewMessageReceived);
NL_TEST_ASSERT(aSuite, delegate.mLastMessageWasStatus);
NL_TEST_ASSERT(aSuite, delegate.mStatus.mStatus != Status::UnsupportedAccess);
@@ -170,6 +174,8 @@
CHIP_ERROR err = exchange->SendMessage(MsgType::TimedRequest, std::move(payload), SendMessageFlags::kExpectResponse);
NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
NL_TEST_ASSERT(aSuite, delegate.mNewMessageReceived);
NL_TEST_ASSERT(aSuite, delegate.mLastMessageWasStatus);
NL_TEST_ASSERT(aSuite, delegate.mStatus.mStatus == Status::Success);
@@ -185,8 +191,10 @@
delegate.mKeepExchangeOpen = false;
delegate.mNewMessageReceived = false;
- err = exchange->SendMessage(aMsgType, std::move(payload));
+ err = exchange->SendMessage(aMsgType, std::move(payload), SendMessageFlags::kExpectResponse);
NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
NL_TEST_ASSERT(aSuite, delegate.mNewMessageReceived);
NL_TEST_ASSERT(aSuite, delegate.mLastMessageWasStatus);
NL_TEST_ASSERT(aSuite, delegate.mStatus.mStatus == Status::UnsupportedAccess);
@@ -217,6 +225,8 @@
CHIP_ERROR err = exchange->SendMessage(MsgType::TimedRequest, std::move(payload), SendMessageFlags::kExpectResponse);
NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
NL_TEST_ASSERT(aSuite, delegate.mNewMessageReceived);
NL_TEST_ASSERT(aSuite, delegate.mLastMessageWasStatus);
NL_TEST_ASSERT(aSuite, delegate.mStatus.mStatus == Status::Success);
@@ -249,7 +259,7 @@
{
"TestTimedHandler",
&sTests[0],
- TestContext::Initialize,
+ TestContext::InitializeAsync,
TestContext::Finalize
};
// clang-format on
