Skip to content

Commit 1209087

Browse files
arkqbzbarsky-apple
authored andcommitted
Fix event logging test for platforms using system time (#27114)
* 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 <[email protected]> --------- Co-authored-by: Boris Zbarsky <[email protected]>
1 parent a82db8c commit 1209087

File tree

3 files changed

+22
-16
lines changed

3 files changed

+22
-16
lines changed

src/app/EventManagement.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ CHIP_ERROR EventManagement::EnsureSpaceInCircularBuffer(size_t aRequiredSpace, P
173173

174174
// Check that we have this much space in all our event buffers that might
175175
// hold the event. If we do not, that will prevent the event from being
176-
// properly evicted into higher-priority bufers. We want to discover
176+
// properly evicted into higher-priority buffers. We want to discover
177177
// this early, so that testing surfaces the need to make those buffers
178178
// larger.
179179
for (auto * currentBuffer = mpEventBuffer; currentBuffer; currentBuffer = currentBuffer->GetNextCircularEventBuffer())

src/app/EventManagement.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -399,9 +399,9 @@ class EventManagement
399399
int mFieldsToRead = 0;
400400
/* PriorityLevel and DeltaTime are there if that is not first event when putting events in report*/
401401
#if CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
402-
Timestamp mCurrentTime = Timestamp::System(System::Clock::kZero);
403-
#else // CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
404402
Timestamp mCurrentTime = Timestamp::Epoch(System::Clock::kZero);
403+
#else // CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
404+
Timestamp mCurrentTime = Timestamp::System(System::Clock::kZero);
405405
#endif // CHIP_DEVICE_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS
406406
PriorityLevel mPriority = PriorityLevel::First;
407407
ClusterId mClusterId = 0;

src/app/tests/TestEventLogging.cpp

+19-13
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ static const chip::EndpointId kTestEndpointId1 = 2;
5555
static const chip::EndpointId kTestEndpointId2 = 3;
5656
static const chip::TLV::Tag kLivenessDeviceStatus = chip::TLV::ContextTag(1);
5757

58-
static uint8_t gDebugEventBuffer[128];
59-
static uint8_t gInfoEventBuffer[128];
60-
static uint8_t gCritEventBuffer[128];
58+
static uint8_t gDebugEventBuffer[120];
59+
static uint8_t gInfoEventBuffer[120];
60+
static uint8_t gCritEventBuffer[120];
6161
static chip::app::CircularEventBuffer gCircularEventBuffer[3];
6262

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

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

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

314-
const nlTest sTests[] = { NL_TEST_DEF("CheckLogEventWithEvictToNextBuffer", CheckLogEventWithEvictToNextBuffer),
315-
NL_TEST_DEF("CheckLogEventWithDiscardLowEvent", CheckLogEventWithDiscardLowEvent), NL_TEST_SENTINEL() };
320+
const nlTest sTests[] = {
321+
NL_TEST_DEF("CheckLogEventWithEvictToNextBuffer", CheckLogEventWithEvictToNextBuffer),
322+
NL_TEST_DEF("CheckLogEventWithDiscardLowEvent", CheckLogEventWithDiscardLowEvent),
323+
NL_TEST_SENTINEL(),
324+
};
316325

317-
// clang-format off
318-
nlTestSuite sSuite =
319-
{
326+
nlTestSuite sSuite = {
320327
"EventLogging",
321328
&sTests[0],
322329
TestContext::Initialize,
323-
TestContext::Finalize
330+
TestContext::Finalize,
324331
};
325-
// clang-format on
326332

327333
} // namespace
328334

0 commit comments

Comments
 (0)