From d00fdce8e91cbbf35509043b494a379d6d7b11ed Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 26 Jul 2023 15:50:33 -0400 Subject: [PATCH 1/6] Removed reportScheduler test flags and made TestReadInteractin.cpp wait for min/max instead of setting flags. Modified subscription times in the test to minimise the impact of waiting. --- src/app/reporting/ReportScheduler.h | 49 +------ src/app/tests/TestReadInteraction.cpp | 191 ++++++++++++++++---------- 2 files changed, 119 insertions(+), 121 deletions(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index fc1858862bc4bd..068237780b86a0 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -63,17 +63,6 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver class ReadHandlerNode : public TimerContext { public: -#ifdef CONFIG_BUILD_FOR_HOST_UNIT_TEST - /// Test flags to allow TestReadInteraction to simulate expiration of the minimal and maximal intervals without - /// waiting - enum class TestFlags : uint8_t{ - MinIntervalElapsed = (1 << 0), - MaxIntervalElapsed = (1 << 1), - }; - void SetTestFlags(TestFlags aFlag, bool aValue) { mFlags.Set(aFlag, aValue); } - bool GetTestFlags(TestFlags aFlag) const { return mFlags.Has(aFlag); } -#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST - ReadHandlerNode(ReadHandler * aReadHandler, TimerDelegate * aTimerDelegate, ReportScheduler * aScheduler) : mTimerDelegate(aTimerDelegate), mScheduler(aScheduler) { @@ -93,29 +82,12 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver { Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); -#ifdef CONFIG_BUILD_FOR_HOST_UNIT_TEST - return (mReadHandler->IsGeneratingReports() && (now >= mMinTimestamp || mFlags.Has(TestFlags::MinIntervalElapsed)) && - (mReadHandler->IsDirty() || (now >= mMaxTimestamp || mFlags.Has(TestFlags::MaxIntervalElapsed)) || - now >= mSyncTimestamp)); -#else return (mReadHandler->IsGeneratingReports() && (now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || now >= mSyncTimestamp))); -#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST } bool IsEngineRunScheduled() const { return mEngineRunScheduled; } - void SetEngineRunScheduled(bool aEngineRunScheduled) - { - mEngineRunScheduled = aEngineRunScheduled; -#ifdef CONFIG_BUILD_FOR_HOST_UNIT_TEST - // If the engine becomes unscheduled, this means a run just took place so we reset the test flags - if (!mEngineRunScheduled) - { - mFlags.Set(TestFlags::MinIntervalElapsed, false); - mFlags.Set(TestFlags::MaxIntervalElapsed, false); - } -#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST - } + void SetEngineRunScheduled(bool aEngineRunScheduled) { mEngineRunScheduled = aEngineRunScheduled; } void SetIntervalTimeStamps(ReadHandler * aReadHandler) { @@ -146,9 +118,6 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver System::Clock::Timestamp GetSyncTimestamp() const { return mSyncTimestamp; } private: -#ifdef CONFIG_BUILD_FOR_HOST_UNIT_TEST - BitFlags mFlags; -#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST TimerDelegate * mTimerDelegate; ReadHandler * mReadHandler; ReportScheduler * mScheduler; @@ -180,22 +149,6 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver size_t GetNumReadHandlers() const { return mNodesPool.Allocated(); } #ifdef CONFIG_BUILD_FOR_HOST_UNIT_TEST - void RunNodeCallbackForHandler(const ReadHandler * aReadHandler) - { - ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); - node->TimerFired(); - } - void SetFlagsForHandler(const ReadHandler * aReadHandler, ReadHandlerNode::TestFlags aFlag, bool aValue) - { - ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); - node->SetTestFlags(aFlag, aValue); - } - - bool CheckFlagsForHandler(const ReadHandler * aReadHandler, ReadHandlerNode::TestFlags aFlag) - { - ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); - return node->GetTestFlags(aFlag); - } Timestamp GetMinTimestampForHandler(const ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index a818ee3d23e3a4..a1563feb4a8f0d 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -1532,8 +1532,8 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a readPrepareParams.mAttributePathParamsListSize = 2; - readPrepareParams.mMinIntervalFloorSeconds = 2; - readPrepareParams.mMaxIntervalCeilingSeconds = 5; + readPrepareParams.mMinIntervalFloorSeconds = 1; + readPrepareParams.mMaxIntervalCeilingSeconds = 2; printf("\nSend first subscribe request message to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId); { @@ -1597,7 +1597,19 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a dirtyPath5.mAttributeId = 4; // Test report with 2 different path - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); + + // Wait for min interval to elapse + System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } + delegate.mGotReport = false; delegate.mGotEventResponse = false; delegate.mNumAttributeResponse = 0; @@ -1609,15 +1621,22 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse == true); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test report with 2 different path, and 1 same path - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); + // Wait for min interval to elapse + startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -1629,14 +1648,21 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test report with 3 different path, and one path is overlapped with another - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); + // Wait for min interval to elapse + startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -1648,14 +1674,21 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test report with 3 different path, all are not overlapped, one path is not interested for current subscription - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); + // Wait for min interval to elapse + startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -1667,18 +1700,21 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test empty report - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MaxIntervalElapsed, true); - // Manually trigger the callback that would schedule the next report as it would normally have been called if the time had - // elapsed as simulated above - reportScheduler->RunNodeCallbackForHandler(delegate.mpReadHandler); + // Wait for max interval to elapse + startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); delegate.mGotReport = false; @@ -1731,7 +1767,7 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite readPrepareParams.mpAttributePathParamsList = nullptr; readPrepareParams.mAttributePathParamsListSize = 0; - readPrepareParams.mMinIntervalFloorSeconds = 2; + readPrepareParams.mMinIntervalFloorSeconds = 1; readPrepareParams.mMaxIntervalCeilingSeconds = 3600; printf("\nSend first subscribe request message with wildcard urgent event to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId); @@ -1784,7 +1820,7 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite // it shouldn't. Better fix could happen inside DriveIOUntil, not sure the sideeffect there. while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(1600)) + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(600)) { break; } @@ -1991,7 +2027,6 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap // Set a concrete path dirty { - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; @@ -2005,9 +2040,6 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); // We subscribed wildcard path twice, so we will receive two reports here. NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); @@ -2015,7 +2047,6 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap // Set a endpoint dirty { - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; delegate.mNumArrayItems = 0; @@ -2037,10 +2068,6 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap ctx.DrainAndServiceIO(); } while (last != delegate.mNumAttributeResponse); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); - NL_TEST_ASSERT(apSuite, delegate.mGotReport); // Mock endpoint3 has 13 attributes in total, and we subscribed twice. // And attribute 3/2/4 is a list with 6 elements and list chunking // is applied to it, but the way the packet boundaries fall we get two of @@ -2111,7 +2138,6 @@ void TestReadInteraction::TestSubscribePartialOverlap(nlTestSuite * apSuite, voi // Set a partial overlapped path dirty { - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; @@ -2124,9 +2150,6 @@ void TestReadInteraction::TestSubscribePartialOverlap(nlTestSuite * apSuite, voi ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 1); NL_TEST_ASSERT(apSuite, delegate.mReceivedAttributePaths[0].mEndpointId == Test::kMockEndpoint2); @@ -2192,7 +2215,6 @@ void TestReadInteraction::TestSubscribeSetDirtyFullyOverlap(nlTestSuite * apSuit // Set a full overlapped path dirty and expect to receive one E2C3A1 { - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; @@ -2202,9 +2224,6 @@ void TestReadInteraction::TestSubscribeSetDirtyFullyOverlap(nlTestSuite * apSuit ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 1); NL_TEST_ASSERT(apSuite, delegate.mReceivedAttributePaths[0].mEndpointId == Test::kMockEndpoint2); @@ -2319,11 +2338,19 @@ void TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); delegate.mpReadHandler = engine->ActiveHandlerAt(0); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MaxIntervalElapsed, true); - // Manually trigger the callback that would schedule the next report as it would normally have been called if the time had - // elapsed as simulated above - reportScheduler->RunNodeCallbackForHandler(delegate.mpReadHandler); + // Wait for max interval to elapse + System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } + + NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); ctx.DrainAndServiceIO(); @@ -2462,8 +2489,8 @@ void TestReadInteraction::TestPostSubscribeRoundtripStatusReportTimeout(nlTestSu readPrepareParams.mAttributePathParamsListSize = 2; - readPrepareParams.mMinIntervalFloorSeconds = 2; - readPrepareParams.mMaxIntervalCeilingSeconds = 5; + readPrepareParams.mMinIntervalFloorSeconds = 0; + readPrepareParams.mMaxIntervalCeilingSeconds = 1; delegate.mNumAttributeResponse = 0; readPrepareParams.mKeepSubscriptions = false; @@ -2499,7 +2526,6 @@ void TestReadInteraction::TestPostSubscribeRoundtripStatusReportTimeout(nlTestSu dirtyPath2.mAttributeId = 2; // Test report with 2 different path - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; @@ -2510,14 +2536,20 @@ void TestReadInteraction::TestPostSubscribeRoundtripStatusReportTimeout(nlTestSu ctx.DrainAndServiceIO(); - NL_TEST_ASSERT( - apSuite, - !reportScheduler->CheckFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MaxIntervalElapsed, true); + // Wait for max interval to elapse + System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; ctx.ExpireSessionBobToAlice(); @@ -2835,8 +2867,8 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout(nlT readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = 1; - readPrepareParams.mMinIntervalFloorSeconds = 2; - readPrepareParams.mMaxIntervalCeilingSeconds = 5; + readPrepareParams.mMinIntervalFloorSeconds = 0; + readPrepareParams.mMaxIntervalCeilingSeconds = 1; delegate.mNumAttributeResponse = 0; readPrepareParams.mKeepSubscriptions = false; @@ -2865,8 +2897,17 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout(nlT dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MaxIntervalElapsed, true); + System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } + err = engine->GetReportingEngine().SetDirty(dirtyPath1); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); delegate.mGotReport = false; @@ -2938,8 +2979,8 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout(nlTestSui readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = 1; - readPrepareParams.mMinIntervalFloorSeconds = 2; - readPrepareParams.mMaxIntervalCeilingSeconds = 5; + readPrepareParams.mMinIntervalFloorSeconds = 0; + readPrepareParams.mMaxIntervalCeilingSeconds = 1; delegate.mNumAttributeResponse = 0; readPrepareParams.mKeepSubscriptions = false; @@ -2968,8 +3009,16 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout(nlTestSui dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); - reportScheduler->SetFlagsForHandler(delegate.mpReadHandler, ReadHandlerNode::TestFlags::MaxIntervalElapsed, true); + System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + while (true) + { + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= + System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) + { + break; + } + ctx.GetIOContext().DriveIO(); + } err = engine->GetReportingEngine().SetDirty(dirtyPath1); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); delegate.mGotReport = false; @@ -3039,7 +3088,7 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReport(nlTestSuite * ap readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = 1; - readPrepareParams.mMinIntervalFloorSeconds = 2; + readPrepareParams.mMinIntervalFloorSeconds = 1; readPrepareParams.mMaxIntervalCeilingSeconds = 5; delegate.mNumAttributeResponse = 0; @@ -3074,12 +3123,12 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReport(nlTestSuite * ap delegate.mNumAttributeResponse = 0; delegate.mNumArrayItems = 0; - // wait for min interval 2 seconds(in test, we use 1.9second considering the time variation), expect no event is received, + // wait for min interval 1 seconds(in test, we use 0.9second considering the time variation), expect no event is received, // then wait for 0.5 seconds, then all chunked dirty reports are sent out, which would not honor minInterval System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(1900)) + if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(900)) { break; } @@ -3573,7 +3622,7 @@ void TestReadInteraction::TestSubscribeClientReceiveUnsolicitedInvalidReportMess ctx.GetLoopback().mSentMessageCount = 0; auto exchange = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext( - delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); + delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); delegate.mpReadHandler->mExchangeCtx.Grab(exchange); err = delegate.mpReadHandler->mExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReportData, std::move(msgBuf), Messaging::SendMessageFlags::kExpectResponse); @@ -3742,7 +3791,7 @@ void TestReadInteraction::TestSubscribeClientReceiveUnsolicitedReportMessageWith ctx.GetLoopback().mSentMessageCount = 0; auto exchange = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext( - delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); + delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); delegate.mpReadHandler->mExchangeCtx.Grab(exchange); err = delegate.mpReadHandler->mExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReportData, std::move(msgBuf), Messaging::SendMessageFlags::kExpectResponse); @@ -4350,14 +4399,11 @@ void TestReadInteraction::TestSubscriptionReportWithDefunctSession(nlTestSuite * NL_TEST_ASSERT(apSuite, SessionHandle(*readHandler->GetSession()) == ctx.GetSessionAliceToBob()); // Test that we send reports as needed. - reportScheduler->SetFlagsForHandler(readHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; engine->GetReportingEngine().SetDirty(subscribePath); - ctx.DrainAndServiceIO(); - NL_TEST_ASSERT(apSuite, - !reportScheduler->CheckFlagsForHandler(readHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed)); + NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 1); NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 1); @@ -4367,7 +4413,6 @@ void TestReadInteraction::TestSubscriptionReportWithDefunctSession(nlTestSuite * // Test that if the session is defunct we don't send reports and clean // up properly. readHandler->GetSession()->MarkAsDefunct(); - reportScheduler->SetFlagsForHandler(readHandler, ReadHandlerNode::TestFlags::MinIntervalElapsed, true); delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; engine->GetReportingEngine().SetDirty(subscribePath); From c7fb1d1e9f696eb0bb9a9a75bb00fd7394bacf5c Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 31 Jul 2023 20:55:05 +0000 Subject: [PATCH 2/6] Restyled by clang-format --- src/app/tests/TestReadInteraction.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index a1563feb4a8f0d..0f3462ea0bec5f 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -3622,7 +3622,7 @@ void TestReadInteraction::TestSubscribeClientReceiveUnsolicitedInvalidReportMess ctx.GetLoopback().mSentMessageCount = 0; auto exchange = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext( - delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); + delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); delegate.mpReadHandler->mExchangeCtx.Grab(exchange); err = delegate.mpReadHandler->mExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReportData, std::move(msgBuf), Messaging::SendMessageFlags::kExpectResponse); @@ -3791,7 +3791,7 @@ void TestReadInteraction::TestSubscribeClientReceiveUnsolicitedReportMessageWith ctx.GetLoopback().mSentMessageCount = 0; auto exchange = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext( - delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); + delegate.mpReadHandler->mSessionHandle.Get().Value(), delegate.mpReadHandler); delegate.mpReadHandler->mExchangeCtx.Grab(exchange); err = delegate.mpReadHandler->mExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReportData, std::move(msgBuf), Messaging::SendMessageFlags::kExpectResponse); From 1f56ddb8682ef564cedce3e2d0456a75a10d1ba8 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 1 Aug 2023 11:40:53 -0400 Subject: [PATCH 3/6] Added a driveAndServiceIO after waiting for max in test where we should wait for max to expire to ensure the run gets scheduled reliably --- src/app/tests/TestReadInteraction.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 0f3462ea0bec5f..f835c50f359736 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -1713,8 +1713,8 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a { break; } - ctx.GetIOContext().DriveIO(); } + ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); delegate.mGotReport = false; @@ -2347,8 +2347,8 @@ void TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite { break; } - ctx.GetIOContext().DriveIO(); } + ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); From 146cc018aae7eb715b06b98b87ab58763d222201 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 1 Aug 2023 16:11:06 -0400 Subject: [PATCH 4/6] Moved ctx.GetIOContext().DriveIO(); after each tests to garantee this will run after maxInterval is expired --- src/app/tests/TestReadInteraction.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index f835c50f359736..60980d2cf823a9 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -2548,8 +2548,9 @@ void TestReadInteraction::TestPostSubscribeRoundtripStatusReportTimeout(nlTestSu { break; } - ctx.GetIOContext().DriveIO(); } + ctx.GetIOContext().DriveIO(); + delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; ctx.ExpireSessionBobToAlice(); @@ -2905,8 +2906,8 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout(nlT { break; } - ctx.GetIOContext().DriveIO(); } + ctx.GetIOContext().DriveIO(); err = engine->GetReportingEngine().SetDirty(dirtyPath1); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -3017,8 +3018,9 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout(nlTestSui { break; } - ctx.GetIOContext().DriveIO(); } + ctx.GetIOContext().DriveIO(); + err = engine->GetReportingEngine().SetDirty(dirtyPath1); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); delegate.mGotReport = false; From 5c605c3a51598dcc1581e2a18d25379ead32dad4 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 2 Aug 2023 13:50:23 -0400 Subject: [PATCH 5/6] Implemented a mock clock in TestReadInteraction to reduce wait loops --- src/app/tests/TestReadInteraction.cpp | 176 +++++++++----------------- 1 file changed, 57 insertions(+), 119 deletions(-) diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 60980d2cf823a9..9ccaf97751aaf2 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -67,11 +67,17 @@ chip::EndpointId kInvalidTestEndpointId = 3; chip::DataVersion kTestDataVersion1 = 3; chip::DataVersion kTestDataVersion2 = 5; +static chip::System::Clock::Internal::MockClock mockClock; +static chip::System::Clock::ClockBase * realClock; + class TestContext : public chip::Test::AppContext { public: static int Initialize(void * context) { + realClock = &chip::System::SystemClock(); + chip::System::Clock::Internal::SetSystemClockForTesting(&mockClock); + if (AppContext::Initialize(context) != SUCCESS) return FAILURE; @@ -97,6 +103,7 @@ class TestContext : public chip::Test::AppContext static int Finalize(void * context) { chip::app::EventManagement::DestroyEventManagement(); + chip::System::Clock::Internal::SetSystemClockForTesting(realClock); if (AppContext::Finalize(context) != SUCCESS) return FAILURE; @@ -1598,17 +1605,9 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a // Test report with 2 different path - // Wait for min interval to elapse - System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) - { - break; - } - ctx.GetIOContext().DriveIO(); - } + // Advance monotonic timestamp for min interval to elapse + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + ctx.GetIOContext().DriveIO(); delegate.mGotReport = false; delegate.mGotEventResponse = false; @@ -1626,17 +1625,10 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test report with 2 different path, and 1 same path - // Wait for min interval to elapse - startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) - { - break; - } - ctx.GetIOContext().DriveIO(); - } + // Advance monotonic timestamp for min interval to elapse + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + ctx.GetIOContext().DriveIO(); + delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -1652,17 +1644,10 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test report with 3 different path, and one path is overlapped with another - // Wait for min interval to elapse - startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) - { - break; - } - ctx.GetIOContext().DriveIO(); - } + // Advance monotonic timestamp for min interval to elapse + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + ctx.GetIOContext().DriveIO(); + delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -1678,17 +1663,10 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test report with 3 different path, all are not overlapped, one path is not interested for current subscription - // Wait for min interval to elapse - startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)) - { - break; - } - ctx.GetIOContext().DriveIO(); - } + // Advance monotonic timestamp for min interval to elapse + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + ctx.GetIOContext().DriveIO(); + delegate.mGotReport = false; delegate.mNumAttributeResponse = 0; err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -1704,16 +1682,8 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Test empty report - // Wait for max interval to elapse - startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) - { - break; - } - } + // Advance monotonic timestamp for min interval to elapse + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); @@ -1788,8 +1758,7 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); ctx.DrainAndServiceIO(); - - System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + System::Clock::Timestamp startTime = mockClock.GetMonotonicTimestamp(); NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 2); NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); @@ -1813,40 +1782,47 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite nonUrgentDelegate.mGotEventResponse = false; nonUrgentDelegate.mGotReport = false; - // wait for min interval 2 seconds (in test, we use 1.6 seconds considering the time variation), expect no event is + // wait for min interval 1 seconds (in test, we use 0.6 seconds considering the time variation), expect no event is // received, then wait for 0.8 seconds, then the urgent event would be sent out // currently DriveIOUntil will call `DriveIO` at least once, which means that if there is any CPU scheduling issues, // there's a chance 1.9s will already have elapsed by the time we get there, which will result in DriveIO being called when // it shouldn't. Better fix could happen inside DriveIOUntil, not sure the sideeffect there. + + // Advance monotonic looping to allow events to trigger while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(600)) + ctx.GetIOContext().DriveIO(); + if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(600)) { break; } - ctx.GetIOContext().DriveIO(); // at least one IO loop is guaranteed + mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); } + ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); - startTime = System::SystemClock().GetMonotonicTimestamp(); + // Advance monotonic timestamp for min interval to elapse + startTime = mockClock.GetMonotonicTimestamp(); while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(800)) + ctx.GetIOContext().DriveIO(); + if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(800)) { break; } - ctx.GetIOContext().DriveIO(); // at least one IO loop is guaranteed + mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); } + ctx.GetIOContext().DriveIO(); + NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); // Since we just sent a report for our urgent subscription, the min interval of the urgent subcription should have been // updated NL_TEST_ASSERT(apSuite, - reportScheduler->GetMinTimestampForHandler(delegate.mpReadHandler) > - System::SystemClock().GetMonotonicTimestamp()); + reportScheduler->GetMinTimestampForHandler(delegate.mpReadHandler) > mockClock.GetMonotonicTimestamp()); NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->IsDirty()); delegate.mGotEventResponse = false; @@ -1854,19 +1830,12 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite // should be in the past NL_TEST_ASSERT(apSuite, reportScheduler->GetMinTimestampForHandler(nonUrgentDelegate.mpReadHandler) < - System::SystemClock().GetMonotonicTimestamp()); + mockClock.GetMonotonicTimestamp()); NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); - // Wait for the min interval timer to fire. - startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(2100)) - { - break; - } - ctx.GetIOContext().DriveIO(); // at least one IO loop is guaranteed - } + // Advance monotonic timestamp for min interval to elapse + mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(2100)); + ctx.GetIOContext().DriveIO(); // No reporting should have happened. NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); @@ -2338,16 +2307,8 @@ void TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); delegate.mpReadHandler = engine->ActiveHandlerAt(0); - // Wait for max interval to elapse - System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) - { - break; - } - } + // Advance monotonic timestamp for min interval to elapse + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); @@ -2540,15 +2501,7 @@ void TestReadInteraction::TestPostSubscribeRoundtripStatusReportTimeout(nlTestSu NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Wait for max interval to elapse - System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) - { - break; - } - } + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); delegate.mGotReport = false; @@ -2898,15 +2851,7 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout(nlT dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) - { - break; - } - } + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -3010,15 +2955,7 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout(nlTestSui dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); - while (true) - { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= - System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)) - { - break; - } - } + mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -3127,26 +3064,27 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReport(nlTestSuite * ap // wait for min interval 1 seconds(in test, we use 0.9second considering the time variation), expect no event is received, // then wait for 0.5 seconds, then all chunked dirty reports are sent out, which would not honor minInterval - System::Clock::Timestamp startTime = System::SystemClock().GetMonotonicTimestamp(); + System::Clock::Timestamp startTime = mockClock.GetMonotonicTimestamp(); while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(900)) + ctx.GetIOContext().DriveIO(); + if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(900)) { break; } - ctx.GetIOContext().DriveIO(); + mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); } NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 0); - startTime = System::SystemClock().GetMonotonicTimestamp(); + startTime = mockClock.GetMonotonicTimestamp(); while (true) { - if ((System::SystemClock().GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(500)) + ctx.GetIOContext().DriveIO(); + if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(500)) { break; } - ctx.GetIOContext().DriveIO(); + mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); } - ctx.DrainAndServiceIO(); } // Two chunked reports carry 4 attributeDataIB: 1 with a list of 3 items, // and then one per remaining item. From e7fe784ca79561edbd1168e8891e3659e2b6a5b5 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 2 Aug 2023 16:39:14 -0400 Subject: [PATCH 6/6] Removed more loops and added comment on loop left in the code --- src/app/tests/TestReadInteraction.cpp | 88 ++++++++++++--------------- 1 file changed, 38 insertions(+), 50 deletions(-) diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 9ccaf97751aaf2..748a47e44fe323 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -67,16 +67,16 @@ chip::EndpointId kInvalidTestEndpointId = 3; chip::DataVersion kTestDataVersion1 = 3; chip::DataVersion kTestDataVersion2 = 5; -static chip::System::Clock::Internal::MockClock mockClock; -static chip::System::Clock::ClockBase * realClock; +static chip::System::Clock::Internal::MockClock gMockClock; +static chip::System::Clock::ClockBase * gRealClock; class TestContext : public chip::Test::AppContext { public: static int Initialize(void * context) { - realClock = &chip::System::SystemClock(); - chip::System::Clock::Internal::SetSystemClockForTesting(&mockClock); + gRealClock = &chip::System::SystemClock(); + chip::System::Clock::Internal::SetSystemClockForTesting(&gMockClock); if (AppContext::Initialize(context) != SUCCESS) return FAILURE; @@ -103,7 +103,7 @@ class TestContext : public chip::Test::AppContext static int Finalize(void * context) { chip::app::EventManagement::DestroyEventManagement(); - chip::System::Clock::Internal::SetSystemClockForTesting(realClock); + chip::System::Clock::Internal::SetSystemClockForTesting(gRealClock); if (AppContext::Finalize(context) != SUCCESS) return FAILURE; @@ -1606,7 +1606,7 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a // Test report with 2 different path // Advance monotonic timestamp for min interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); ctx.GetIOContext().DriveIO(); delegate.mGotReport = false; @@ -1626,7 +1626,7 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a // Test report with 2 different path, and 1 same path // Advance monotonic timestamp for min interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); ctx.GetIOContext().DriveIO(); delegate.mGotReport = false; @@ -1645,7 +1645,7 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a // Test report with 3 different path, and one path is overlapped with another // Advance monotonic timestamp for min interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); ctx.GetIOContext().DriveIO(); delegate.mGotReport = false; @@ -1664,7 +1664,7 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a // Test report with 3 different path, all are not overlapped, one path is not interested for current subscription // Advance monotonic timestamp for min interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMinIntervalFloorSeconds)); ctx.GetIOContext().DriveIO(); delegate.mGotReport = false; @@ -1683,7 +1683,7 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a // Test empty report // Advance monotonic timestamp for min interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); @@ -1758,7 +1758,7 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); ctx.DrainAndServiceIO(); - System::Clock::Timestamp startTime = mockClock.GetMonotonicTimestamp(); + System::Clock::Timestamp startTime = gMockClock.GetMonotonicTimestamp(); NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers() == 2); NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr); @@ -1789,31 +1789,23 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite // it shouldn't. Better fix could happen inside DriveIOUntil, not sure the sideeffect there. // Advance monotonic looping to allow events to trigger - while (true) - { - ctx.GetIOContext().DriveIO(); - if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(600)) - { - break; - } - mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); - } + gMockClock.AdvanceMonotonic(System::Clock::Milliseconds32(600)); ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mGotEventResponse); // Advance monotonic timestamp for min interval to elapse - startTime = mockClock.GetMonotonicTimestamp(); - while (true) - { - ctx.GetIOContext().DriveIO(); - if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(800)) - { - break; - } - mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); - } + startTime = gMockClock.GetMonotonicTimestamp(); + gMockClock.AdvanceMonotonic(System::Clock::Milliseconds32(800)); + + // Service Timer expired event + ctx.GetIOContext().DriveIO(); + + // Service Engine Run + ctx.GetIOContext().DriveIO(); + + // Service EventManagement event ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); @@ -1822,7 +1814,7 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite // Since we just sent a report for our urgent subscription, the min interval of the urgent subcription should have been // updated NL_TEST_ASSERT(apSuite, - reportScheduler->GetMinTimestampForHandler(delegate.mpReadHandler) > mockClock.GetMonotonicTimestamp()); + reportScheduler->GetMinTimestampForHandler(delegate.mpReadHandler) > gMockClock.GetMonotonicTimestamp()); NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->IsDirty()); delegate.mGotEventResponse = false; @@ -1830,11 +1822,11 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite // should be in the past NL_TEST_ASSERT(apSuite, reportScheduler->GetMinTimestampForHandler(nonUrgentDelegate.mpReadHandler) < - mockClock.GetMonotonicTimestamp()); + gMockClock.GetMonotonicTimestamp()); NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); // Advance monotonic timestamp for min interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(2100)); + gMockClock.AdvanceMonotonic(System::Clock::Milliseconds32(2100)); ctx.GetIOContext().DriveIO(); // No reporting should have happened. @@ -2308,7 +2300,7 @@ void TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite delegate.mpReadHandler = engine->ActiveHandlerAt(0); // Advance monotonic timestamp for min interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); NL_TEST_ASSERT(apSuite, engine->GetReportingEngine().IsRunScheduled()); @@ -2501,7 +2493,7 @@ void TestReadInteraction::TestPostSubscribeRoundtripStatusReportTimeout(nlTestSu NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); // Wait for max interval to elapse - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); delegate.mGotReport = false; @@ -2851,7 +2843,7 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkStatusReportTimeout(nlT dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -2955,7 +2947,7 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReportTimeout(nlTestSui dirtyPath1.mEndpointId = Test::kMockEndpoint3; dirtyPath1.mAttributeId = Test::MockAttributeId(4); - mockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); + gMockClock.AdvanceMonotonic(System::Clock::Seconds16(readPrepareParams.mMaxIntervalCeilingSeconds)); ctx.GetIOContext().DriveIO(); err = engine->GetReportingEngine().SetDirty(dirtyPath1); @@ -3064,26 +3056,22 @@ void TestReadInteraction::TestPostSubscribeRoundtripChunkReport(nlTestSuite * ap // wait for min interval 1 seconds(in test, we use 0.9second considering the time variation), expect no event is received, // then wait for 0.5 seconds, then all chunked dirty reports are sent out, which would not honor minInterval - System::Clock::Timestamp startTime = mockClock.GetMonotonicTimestamp(); - while (true) - { - ctx.GetIOContext().DriveIO(); - if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(900)) - { - break; - } - mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); - } + gMockClock.AdvanceMonotonic(System::Clock::Milliseconds32(900)); + ctx.GetIOContext().DriveIO(); + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 0); - startTime = mockClock.GetMonotonicTimestamp(); + System::Clock::Timestamp startTime = gMockClock.GetMonotonicTimestamp(); + + // Increment in time is done by steps here to allow for multiple IO processing at the right time and allow the timer to be + // rescheduled accordingly while (true) { ctx.GetIOContext().DriveIO(); - if ((mockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(500)) + if ((gMockClock.GetMonotonicTimestamp() - startTime) >= System::Clock::Milliseconds32(500)) { break; } - mockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); + gMockClock.AdvanceMonotonic(System::Clock::Milliseconds32(10)); } } // Two chunked reports carry 4 attributeDataIB: 1 with a list of 3 items,