Skip to content

Commit

Permalink
Fix event logging test for platforms using system time (#27114)
Browse files Browse the repository at this point in the history
* Fix event logging test for platforms using system time

Using UTC (epoch) vs system relative time in the event logging affects
the size of TLV-encoded events. This has to be accounted for in the unit
test otherwise the test might fail without apparent reasons.

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
2 people authored and pull[bot] committed Jan 11, 2024
1 parent a59d87f commit 8883749
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/app/EventManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ CHIP_ERROR EventManagement::EnsureSpaceInCircularBuffer(size_t aRequiredSpace, P

// Check that we have this much space in all our event buffers that might
// hold the event. If we do not, that will prevent the event from being
// properly evicted into higher-priority bufers. We want to discover
// properly evicted into higher-priority buffers. We want to discover
// this early, so that testing surfaces the need to make those buffers
// larger.
for (auto * currentBuffer = mpEventBuffer; currentBuffer; currentBuffer = currentBuffer->GetNextCircularEventBuffer())
Expand Down
4 changes: 2 additions & 2 deletions src/app/EventManagement.h
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,9 @@ class EventManagement
int mFieldsToRead = 0;
/* PriorityLevel and DeltaTime are there if that is not first event when putting events in report*/
#if CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
Timestamp mCurrentTime = Timestamp::System(System::Clock::kZero);
#else // CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
Timestamp mCurrentTime = Timestamp::Epoch(System::Clock::kZero);
#else // CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
Timestamp mCurrentTime = Timestamp::System(System::Clock::kZero);
#endif // CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
PriorityLevel mPriority = PriorityLevel::First;
ClusterId mClusterId = 0;
Expand Down
32 changes: 19 additions & 13 deletions src/app/tests/TestEventLogging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ static const chip::EndpointId kTestEndpointId1 = 2;
static const chip::EndpointId kTestEndpointId2 = 3;
static const chip::TLV::Tag kLivenessDeviceStatus = chip::TLV::ContextTag(1);

static uint8_t gDebugEventBuffer[128];
static uint8_t gInfoEventBuffer[128];
static uint8_t gCritEventBuffer[128];
static uint8_t gDebugEventBuffer[120];
static uint8_t gInfoEventBuffer[120];
static uint8_t gCritEventBuffer[120];
static chip::app::CircularEventBuffer gCircularEventBuffer[3];

class TestContext : public chip::Test::AppContext
Expand Down Expand Up @@ -154,6 +154,15 @@ static void CheckLogReadOut(nlTestSuite * apSuite, chip::app::EventManagement &
err = alogMgmt.FetchEventsSince(writer, clusterInfo, startingEventNumber, eventCount, chip::Access::SubjectDescriptor{});
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR || err == CHIP_END_OF_TLV);

auto eventTLVSize = writer.GetLengthWritten() / eventCount;
// XXX: Make sure that the sizes of our event storages are big enough to hold at least 3 events
// but small enough not to hold 4 events. It is very important to check this because of the
// hard-coded logic of this unit test.
// The size of TLV-encoded event can vary depending on the UTC vs system time controlled by
// the CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS, because the relative system time
// will be most likely encoded in 1 byte, while the UTC time will be encoded in 8 bytes.
NL_TEST_ASSERT(apSuite, sizeof(gDebugEventBuffer) >= eventTLVSize * 3 && sizeof(gDebugEventBuffer) < eventTLVSize * 4);

reader.Init(backingStore.Get(), writer.GetLengthWritten());

err = chip::TLV::Utilities::Count(reader, totalNumElements, false);
Expand Down Expand Up @@ -307,22 +316,19 @@ static void CheckLogEventWithDiscardLowEvent(nlTestSuite * apSuite, void * apCon
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
CheckLogState(apSuite, logMgmt, 3, chip::app::PriorityLevel::Debug);
}
/**
* Test Suite. It lists all the test functions.
*/

const nlTest sTests[] = { NL_TEST_DEF("CheckLogEventWithEvictToNextBuffer", CheckLogEventWithEvictToNextBuffer),
NL_TEST_DEF("CheckLogEventWithDiscardLowEvent", CheckLogEventWithDiscardLowEvent), NL_TEST_SENTINEL() };
const nlTest sTests[] = {
NL_TEST_DEF("CheckLogEventWithEvictToNextBuffer", CheckLogEventWithEvictToNextBuffer),
NL_TEST_DEF("CheckLogEventWithDiscardLowEvent", CheckLogEventWithDiscardLowEvent),
NL_TEST_SENTINEL(),
};

// clang-format off
nlTestSuite sSuite =
{
nlTestSuite sSuite = {
"EventLogging",
&sTests[0],
TestContext::Initialize,
TestContext::Finalize
TestContext::Finalize,
};
// clang-format on

} // namespace

Expand Down

0 comments on commit 8883749

Please sign in to comment.