Skip to content

Commit

Permalink
Add a unit test of System::Layer timer order (#12895)
Browse files Browse the repository at this point in the history
#### Problem

Nothing explicitly tests that `System::Layer` timers fire in
chronological order. Since we can now have a mock system clock,
this is testable.

#### Change overview

- Add a unit test verifying that timer callbacks run in order.
- Add unit test assertions for timer statistics.
- Revise implementation selection in TestSystemTimer to use the
  `Layer` class rather than only '#if', since not every configuration
  with sockets/LwIP necessarily needs to use the provided `Layer`
  implementations.
- Provide a common MockClock implementation.

#### Testing

Res ipsa loquitur.
  • Loading branch information
kpschoedel authored and pull[bot] committed Feb 19, 2024
1 parent a9e890b commit 1318608
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 64 deletions.
56 changes: 22 additions & 34 deletions src/lib/dnssd/minimal_mdns/tests/TestActiveResolveAttempts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,6 @@ using namespace chip;
using namespace chip::System::Clock::Literals;
using chip::System::Clock::Timeout;

class MockClock : public System::Clock::ClockImpl
{
public:
System::Clock::Microseconds64 GetMonotonicMicroseconds64() override { return mMsec; }
System::Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return mMsec; }

void Advance(System::Clock::Milliseconds32 ms) { mMsec += ms; }

private:
System::Clock::Milliseconds64 mMsec;
};

PeerId MakePeerId(NodeId nodeId)
{
PeerId peerId;
Expand All @@ -46,10 +34,10 @@ PeerId MakePeerId(NodeId nodeId)

void TestSinglePeerAddRemove(nlTestSuite * inSuite, void * inContext)
{
MockClock mockClock;
System::Clock::Internal::MockClock mockClock;
mdns::Minimal::ActiveResolveAttempts attempts(&mockClock);

mockClock.Advance(1234_ms32);
mockClock.AdvanceMonotonic(1234_ms32);

// Starting up, no scheduled peers are expected
NL_TEST_ASSERT(inSuite, !attempts.GetTimeUntilNextExpectedResponse().HasValue());
Expand All @@ -65,20 +53,20 @@ void TestSinglePeerAddRemove(nlTestSuite * inSuite, void * inContext)

// one Next schedule is called, expect to have a delay of 1000 ms
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(1000_ms32));
mockClock.Advance(500_ms32);
mockClock.AdvanceMonotonic(500_ms32);
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(500_ms32));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());

// past due date: timeout should be 0
mockClock.Advance(800_ms32);
mockClock.AdvanceMonotonic(800_ms32);
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(0_ms32));
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(1)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());

// one Next schedule is called, expect to have a delay of 2000 ms
// sincve the timeout doubles every time
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(2000_ms32));
mockClock.Advance(100_ms32);
mockClock.AdvanceMonotonic(100_ms32);
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(1900_ms32));

// once complete, nothing to schedule
Expand All @@ -89,10 +77,10 @@ void TestSinglePeerAddRemove(nlTestSuite * inSuite, void * inContext)

void TestRescheduleSamePeerId(nlTestSuite * inSuite, void * inContext)
{
MockClock mockClock;
System::Clock::Internal::MockClock mockClock;
mdns::Minimal::ActiveResolveAttempts attempts(&mockClock);

mockClock.Advance(112233_ms32);
mockClock.AdvanceMonotonic(112233_ms32);

attempts.MarkPending(MakePeerId(1));

Expand All @@ -104,7 +92,7 @@ void TestRescheduleSamePeerId(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(1000_ms32));

// 2nd try goes to 2 seconds (once at least 1 second passes)
mockClock.Advance(1234_ms32);
mockClock.AdvanceMonotonic(1234_ms32);
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(0_ms32));
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(1)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
Expand All @@ -122,21 +110,21 @@ void TestRescheduleSamePeerId(nlTestSuite * inSuite, void * inContext)
void TestLRU(nlTestSuite * inSuite, void * inContext)
{
// validates that the LRU logic is working
MockClock mockClock;
System::Clock::Internal::MockClock mockClock;
mdns::Minimal::ActiveResolveAttempts attempts(&mockClock);

mockClock.Advance(334455_ms32);
mockClock.AdvanceMonotonic(334455_ms32);

// add a single very old peer
attempts.MarkPending(MakePeerId(9999));
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(9999)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());

mockClock.Advance(1000_ms32);
mockClock.AdvanceMonotonic(1000_ms32);
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(9999)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());

mockClock.Advance(2000_ms32);
mockClock.AdvanceMonotonic(2000_ms32);
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(9999)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());

Expand All @@ -145,7 +133,7 @@ void TestLRU(nlTestSuite * inSuite, void * inContext)
for (uint32_t i = 1; i < mdns::Minimal::ActiveResolveAttempts::kRetryQueueSize; i++)
{
attempts.MarkPending(MakePeerId(i));
mockClock.Advance(1_ms32);
mockClock.AdvanceMonotonic(1_ms32);

NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(i)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
Expand All @@ -159,7 +147,7 @@ void TestLRU(nlTestSuite * inSuite, void * inContext)

// add another element - this should overwrite peer 9999
attempts.MarkPending(MakePeerId(mdns::Minimal::ActiveResolveAttempts::kRetryQueueSize));
mockClock.Advance(32_s16);
mockClock.AdvanceMonotonic(32_s16);

for (Optional<PeerId> peerId = attempts.NextScheduledPeer(); peerId.HasValue(); peerId = attempts.NextScheduledPeer())
{
Expand All @@ -182,7 +170,7 @@ void TestLRU(nlTestSuite * inSuite, void * inContext)
break;
}

mockClock.Advance(ms.Value());
mockClock.AdvanceMonotonic(ms.Value());

Optional<PeerId> peerId = attempts.NextScheduledPeer();
while (peerId.HasValue())
Expand All @@ -196,18 +184,18 @@ void TestLRU(nlTestSuite * inSuite, void * inContext)

void TestNextPeerOrdering(nlTestSuite * inSuite, void * inContext)
{
MockClock mockClock;
System::Clock::Internal::MockClock mockClock;
mdns::Minimal::ActiveResolveAttempts attempts(&mockClock);

mockClock.Advance(123321_ms32);
mockClock.AdvanceMonotonic(123321_ms32);

// add a single peer that will be resolved quickly
attempts.MarkPending(MakePeerId(1));

NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(1)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(1000_ms32));
mockClock.Advance(20_ms32);
mockClock.AdvanceMonotonic(20_ms32);
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(980_ms32));

// expect peerid to be resolve within 1 second from now
Expand All @@ -216,13 +204,13 @@ void TestNextPeerOrdering(nlTestSuite * inSuite, void * inContext)
// mock that we are querying 2 as well
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(2)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
mockClock.Advance(80_ms32);
mockClock.AdvanceMonotonic(80_ms32);
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(900_ms32));

// Peer 1 is done, now peer2 should be pending (in 980ms)
attempts.Complete(MakePeerId(1));
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(920_ms32));
mockClock.Advance(20_ms32);
mockClock.AdvanceMonotonic(20_ms32);
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(900_ms32));

// Once peer 3 is added, queue should be
Expand All @@ -236,14 +224,14 @@ void TestNextPeerOrdering(nlTestSuite * inSuite, void * inContext)
// After the clock advance
// - 400 ms until peer id 2 is pending
// - 500 ms until peer id 3 is pending
mockClock.Advance(500_ms32);
mockClock.AdvanceMonotonic(500_ms32);

NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(400_ms32));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());

// advancing the clock 'too long' will return both other entries, in reverse order due to how
// the internal cache is built
mockClock.Advance(500_ms32);
mockClock.AdvanceMonotonic(500_ms32);
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(3)));
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(2)));
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
Expand Down
30 changes: 30 additions & 0 deletions src/system/SystemClock.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,36 @@ inline void SetSystemClockForTesting(Clock::ClockBase * clock)
Clock::Internal::gClockBase = clock;
}

// Provide a mock implementation for use by unit tests.
class MockClock : public ClockImpl
{
public:
Microseconds64 GetMonotonicMicroseconds64() override { return mSystemTime; }
Milliseconds64 GetMonotonicMilliseconds64() override { return std::chrono::duration_cast<Milliseconds64>(mSystemTime); }
CHIP_ERROR GetClock_RealTime(Microseconds64 & aCurTime) override
{
aCurTime = mRealTime;
return CHIP_NO_ERROR;
}
CHIP_ERROR GetClock_RealTimeMS(Milliseconds64 & aCurTime) override
{
aCurTime = std::chrono::duration_cast<Milliseconds64>(mRealTime);
return CHIP_NO_ERROR;
}
CHIP_ERROR SetClock_RealTime(Microseconds64 aNewCurTime) override
{
mRealTime = aNewCurTime;
return CHIP_NO_ERROR;
}

void SetMonotonic(Milliseconds64 timestamp) { mSystemTime = timestamp; }
void AdvanceMonotonic(Milliseconds64 increment) { mSystemTime += increment; }
void AdvanceRealTime(Milliseconds64 increment) { mRealTime += increment; }

Microseconds64 mSystemTime = Clock::kZero;
Microseconds64 mRealTime = Clock::kZero;
};

} // namespace Internal

#if CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS
Expand Down
6 changes: 6 additions & 0 deletions src/system/SystemStats.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ const Label * GetStrings();
// Additional macros for testing.
#define SYSTEM_STATS_TEST_IN_USE(entry, expected) (chip::System::Stats::GetResourcesInUse()[entry] == (expected))
#define SYSTEM_STATS_TEST_HIGH_WATER_MARK(entry, expected) (chip::System::Stats::GetHighWatermarks()[entry] == (expected))
#define SYSTEM_STATS_RESET_HIGH_WATER_MARK_FOR_TESTING(entry) \
do \
{ \
chip::System::Stats::GetHighWatermarks()[entry] = 0; \
} while (0)

#else // CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS

Expand All @@ -171,5 +176,6 @@ const Label * GetStrings();

#define SYSTEM_STATS_TEST_IN_USE(entry, expected) (true)
#define SYSTEM_STATS_TEST_HIGH_WATER_MARK(entry, expected) (true)
#define SYSTEM_STATS_RESET_HIGH_WATER_MARK_FOR_TESTING(entry)

#endif // CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS
11 changes: 2 additions & 9 deletions src/system/tests/TestSystemClock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,7 @@ void TestRealClock(nlTestSuite * inSuite, void * inContext)

void TestMockClock(nlTestSuite * inSuite, void * inContext)
{
class MockClock : public Clock::ClockImpl
{
public:
Clock::Microseconds64 GetMonotonicMicroseconds64() override { return mTime; }
Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return mTime; }
Clock::Milliseconds64 mTime = Clock::kZero;
};
MockClock clock;
Clock::Internal::MockClock clock;

Clock::ClockBase * savedRealClock = &SystemClock();
Clock::Internal::SetSystemClockForTesting(&clock);
Expand All @@ -101,7 +94,7 @@ void TestMockClock(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMicroseconds64() == Clock::kZero);

constexpr Clock::Milliseconds64 k1234 = Clock::Milliseconds64(1234);
clock.mTime = k1234;
clock.SetMonotonic(k1234);
NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMilliseconds64() == k1234);
NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMicroseconds64() == k1234);

Expand Down
Loading

0 comments on commit 1318608

Please sign in to comment.