Skip to content

Commit

Permalink
Fail unit tests that leave messages sitting around undrained.
Browse files Browse the repository at this point in the history
Tests might be accidentally forgetting to drain their messages, and
hence think messages got delivered when they did not.  Fail the tests
if that happens.

Fixes #17624
  • Loading branch information
bzbarsky-apple committed May 21, 2022
1 parent 55ef609 commit e192d50
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
24 changes: 20 additions & 4 deletions src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ class TestReadInteraction
static void TestSubscribeEarlyShutdown(nlTestSuite * apSuite, void * apContext);
static void TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite * apSuite, void * apContext);
static void TestReadInvalidAttributePathRoundtrip(nlTestSuite * apSuite, void * apContext);
static void TestSubscribeInvalidIterval(nlTestSuite * apSuite, void * apContext);
static void TestSubscribeInvalidInterval(nlTestSuite * apSuite, void * apContext);
static void TestReadShutdown(nlTestSuite * apSuite, void * apContext);
static void TestResubscribeRoundtrip(nlTestSuite * apSuite, void * apContext);
static void TestSubscribeRoundtripStatusReportTimeout(nlTestSuite * apSuite, void * apContext);
Expand Down Expand Up @@ -411,6 +411,12 @@ void TestReadInteraction::TestReadClient(nlTestSuite * apSuite, void * apContext
err = readClient.SendRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

// We don't actually want to deliver that message, because we want to
// synthesize the read response. But we don't want it hanging around
// forever either.
ctx.GetLoopback().mNumMessagesToDrop = 1;
ctx.DrainAndServiceIO();

GenerateReportData(apSuite, apContext, buf, false /*aNeedInvalidReport*/, true /* aSuppressResponse*/);
err = readClient.ProcessReportData(std::move(buf));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
Expand Down Expand Up @@ -545,6 +551,12 @@ void TestReadInteraction::TestReadClientInvalidReport(nlTestSuite * apSuite, voi
err = readClient.SendRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

// We don't actually want to deliver that message, because we want to
// synthesize the read response. But we don't want it hanging around
// forever either.
ctx.GetLoopback().mNumMessagesToDrop = 1;
ctx.DrainAndServiceIO();

GenerateReportData(apSuite, apContext, buf, true /*aNeedInvalidReport*/, true /* aSuppressResponse*/);

err = readClient.ProcessReportData(std::move(buf));
Expand Down Expand Up @@ -1992,7 +2004,7 @@ void TestReadInteraction::TestReadShutdown(nlTestSuite * apSuite, void * apConte
Platform::Delete(pClients[2]);
}

void TestReadInteraction::TestSubscribeInvalidIterval(nlTestSuite * apSuite, void * apContext)
void TestReadInteraction::TestSubscribeInvalidInterval(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -2553,6 +2565,11 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout(nlTestSui
NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0);
engine->Shutdown();
NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);

// Engine shutdown seems to trigger some messages to try to be sent. Make
// sure those get flushed out.
ctx.DrainAndServiceIO();

ctx.CreateSessionBobToAlice();
}
} // namespace app
Expand Down Expand Up @@ -2593,10 +2610,9 @@ const nlTest sTests[] =
NL_TEST_DEF("TestSubscribeEarlyShutdown", chip::app::TestReadInteraction::TestSubscribeEarlyShutdown),
NL_TEST_DEF("TestSubscribeInvalidAttributePathRoundtrip", chip::app::TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip),
NL_TEST_DEF("TestReadInvalidAttributePathRoundtrip", chip::app::TestReadInteraction::TestReadInvalidAttributePathRoundtrip),
NL_TEST_DEF("TestSubscribeInvalidIterval", chip::app::TestReadInteraction::TestSubscribeInvalidIterval),
NL_TEST_DEF("TestSubscribeInvalidInterval", chip::app::TestReadInteraction::TestSubscribeInvalidInterval),
NL_TEST_DEF("TestSubscribeRoundtripStatusReportTimeout", chip::app::TestReadInteraction::TestSubscribeRoundtripStatusReportTimeout),
NL_TEST_DEF("TestPostSubscribeRoundtripStatusReportTimeout", chip::app::TestReadInteraction::TestPostSubscribeRoundtripStatusReportTimeout),
//#if 0
NL_TEST_DEF("TestReadChunkingStatusReportTimeout", chip::app::TestReadInteraction::TestReadChunkingStatusReportTimeout),
NL_TEST_DEF("TestSubscribeRoundtripChunkStatusReportTimeout", chip::app::TestReadInteraction::TestSubscribeRoundtripChunkStatusReportTimeout),
NL_TEST_DEF("TestPostSubscribeRoundtripChunkStatusReportTimeout", chip::app::TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout),
Expand Down
3 changes: 3 additions & 0 deletions src/app/tests/TestReportingEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ void TestReportingEngine::TestBuildAndSendSingleReportData(nlTestSuite * apSuite
app::ReadHandler readHandler(dummy, exchangeCtx, chip::app::ReadHandler::InteractionType::Read);
readHandler.OnInitialRequest(std::move(readRequestbuf));
err = InteractionModelEngine::GetInstance()->GetReportingEngine().BuildAndSendSingleReportData(&readHandler);

ctx.DrainAndServiceIO();

NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
}

Expand Down
7 changes: 3 additions & 4 deletions src/transport/raw/tests/NetworkTestHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,9 @@ class LoopbackTransport : public Transport::Base
void InitLoopbackTransport(System::Layer * systemLayer) { mSystemLayer = systemLayer; }
void ShutdownLoopbackTransport()
{
// TODO: remove these after #17624 (Ensure tests drain all message in loopback transport) being fixed
// Packets are allocated from platform memory, we should release them before Platform::MemoryShutdown
while (!mPendingMessageQueue.empty())
mPendingMessageQueue.pop();
// Make sure no one left packets hanging out that they thought got
// delivered but actually didn't.
VerifyOrDie(mPendingMessageQueue.empty());
}

/// Transports are required to have a constructor that takes exactly one argument
Expand Down

0 comments on commit e192d50

Please sign in to comment.