Skip to content

Commit bb3efe3

Browse files
kpschoedelpull[bot]
authored andcommitted
Add a unit test of System::Layer timer order (#12895)
#### 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.
1 parent 7d338ac commit bb3efe3

File tree

5 files changed

+166
-64
lines changed

5 files changed

+166
-64
lines changed

src/lib/dnssd/minimal_mdns/tests/TestActiveResolveAttempts.cpp

+22-34
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,6 @@ using namespace chip;
2626
using namespace chip::System::Clock::Literals;
2727
using chip::System::Clock::Timeout;
2828

29-
class MockClock : public System::Clock::ClockImpl
30-
{
31-
public:
32-
System::Clock::Microseconds64 GetMonotonicMicroseconds64() override { return mMsec; }
33-
System::Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return mMsec; }
34-
35-
void Advance(System::Clock::Milliseconds32 ms) { mMsec += ms; }
36-
37-
private:
38-
System::Clock::Milliseconds64 mMsec;
39-
};
40-
4129
PeerId MakePeerId(NodeId nodeId)
4230
{
4331
PeerId peerId;
@@ -46,10 +34,10 @@ PeerId MakePeerId(NodeId nodeId)
4634

4735
void TestSinglePeerAddRemove(nlTestSuite * inSuite, void * inContext)
4836
{
49-
MockClock mockClock;
37+
System::Clock::Internal::MockClock mockClock;
5038
mdns::Minimal::ActiveResolveAttempts attempts(&mockClock);
5139

52-
mockClock.Advance(1234_ms32);
40+
mockClock.AdvanceMonotonic(1234_ms32);
5341

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

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

7260
// past due date: timeout should be 0
73-
mockClock.Advance(800_ms32);
61+
mockClock.AdvanceMonotonic(800_ms32);
7462
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(0_ms32));
7563
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(1)));
7664
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
7765

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

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

9078
void TestRescheduleSamePeerId(nlTestSuite * inSuite, void * inContext)
9179
{
92-
MockClock mockClock;
80+
System::Clock::Internal::MockClock mockClock;
9381
mdns::Minimal::ActiveResolveAttempts attempts(&mockClock);
9482

95-
mockClock.Advance(112233_ms32);
83+
mockClock.AdvanceMonotonic(112233_ms32);
9684

9785
attempts.MarkPending(MakePeerId(1));
9886

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

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

128-
mockClock.Advance(334455_ms32);
116+
mockClock.AdvanceMonotonic(334455_ms32);
129117

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

135-
mockClock.Advance(1000_ms32);
123+
mockClock.AdvanceMonotonic(1000_ms32);
136124
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(9999)));
137125
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
138126

139-
mockClock.Advance(2000_ms32);
127+
mockClock.AdvanceMonotonic(2000_ms32);
140128
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(9999)));
141129
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
142130

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

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

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

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

185-
mockClock.Advance(ms.Value());
173+
mockClock.AdvanceMonotonic(ms.Value());
186174

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

197185
void TestNextPeerOrdering(nlTestSuite * inSuite, void * inContext)
198186
{
199-
MockClock mockClock;
187+
System::Clock::Internal::MockClock mockClock;
200188
mdns::Minimal::ActiveResolveAttempts attempts(&mockClock);
201189

202-
mockClock.Advance(123321_ms32);
190+
mockClock.AdvanceMonotonic(123321_ms32);
203191

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

207195
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(1)));
208196
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());
209197
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(1000_ms32));
210-
mockClock.Advance(20_ms32);
198+
mockClock.AdvanceMonotonic(20_ms32);
211199
NL_TEST_ASSERT(inSuite, attempts.GetTimeUntilNextExpectedResponse() == Optional<Timeout>(980_ms32));
212200

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

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

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

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

244232
// advancing the clock 'too long' will return both other entries, in reverse order due to how
245233
// the internal cache is built
246-
mockClock.Advance(500_ms32);
234+
mockClock.AdvanceMonotonic(500_ms32);
247235
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(3)));
248236
NL_TEST_ASSERT(inSuite, attempts.NextScheduledPeer() == Optional<PeerId>::Value(MakePeerId(2)));
249237
NL_TEST_ASSERT(inSuite, !attempts.NextScheduledPeer().HasValue());

src/system/SystemClock.h

+30
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,36 @@ inline void SetSystemClockForTesting(Clock::ClockBase * clock)
303303
Clock::Internal::gClockBase = clock;
304304
}
305305

306+
// Provide a mock implementation for use by unit tests.
307+
class MockClock : public ClockImpl
308+
{
309+
public:
310+
Microseconds64 GetMonotonicMicroseconds64() override { return mSystemTime; }
311+
Milliseconds64 GetMonotonicMilliseconds64() override { return std::chrono::duration_cast<Milliseconds64>(mSystemTime); }
312+
CHIP_ERROR GetClock_RealTime(Microseconds64 & aCurTime) override
313+
{
314+
aCurTime = mRealTime;
315+
return CHIP_NO_ERROR;
316+
}
317+
CHIP_ERROR GetClock_RealTimeMS(Milliseconds64 & aCurTime) override
318+
{
319+
aCurTime = std::chrono::duration_cast<Milliseconds64>(mRealTime);
320+
return CHIP_NO_ERROR;
321+
}
322+
CHIP_ERROR SetClock_RealTime(Microseconds64 aNewCurTime) override
323+
{
324+
mRealTime = aNewCurTime;
325+
return CHIP_NO_ERROR;
326+
}
327+
328+
void SetMonotonic(Milliseconds64 timestamp) { mSystemTime = timestamp; }
329+
void AdvanceMonotonic(Milliseconds64 increment) { mSystemTime += increment; }
330+
void AdvanceRealTime(Milliseconds64 increment) { mRealTime += increment; }
331+
332+
Microseconds64 mSystemTime = Clock::kZero;
333+
Microseconds64 mRealTime = Clock::kZero;
334+
};
335+
306336
} // namespace Internal
307337

308338
#if CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS

src/system/SystemStats.h

+6
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,11 @@ const Label * GetStrings();
156156
// Additional macros for testing.
157157
#define SYSTEM_STATS_TEST_IN_USE(entry, expected) (chip::System::Stats::GetResourcesInUse()[entry] == (expected))
158158
#define SYSTEM_STATS_TEST_HIGH_WATER_MARK(entry, expected) (chip::System::Stats::GetHighWatermarks()[entry] == (expected))
159+
#define SYSTEM_STATS_RESET_HIGH_WATER_MARK_FOR_TESTING(entry) \
160+
do \
161+
{ \
162+
chip::System::Stats::GetHighWatermarks()[entry] = 0; \
163+
} while (0)
159164

160165
#else // CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS
161166

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

172177
#define SYSTEM_STATS_TEST_IN_USE(entry, expected) (true)
173178
#define SYSTEM_STATS_TEST_HIGH_WATER_MARK(entry, expected) (true)
179+
#define SYSTEM_STATS_RESET_HIGH_WATER_MARK_FOR_TESTING(entry)
174180

175181
#endif // CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS

src/system/tests/TestSystemClock.cpp

+2-9
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,7 @@ void TestRealClock(nlTestSuite * inSuite, void * inContext)
8585

8686
void TestMockClock(nlTestSuite * inSuite, void * inContext)
8787
{
88-
class MockClock : public Clock::ClockImpl
89-
{
90-
public:
91-
Clock::Microseconds64 GetMonotonicMicroseconds64() override { return mTime; }
92-
Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return mTime; }
93-
Clock::Milliseconds64 mTime = Clock::kZero;
94-
};
95-
MockClock clock;
88+
Clock::Internal::MockClock clock;
9689

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

10396
constexpr Clock::Milliseconds64 k1234 = Clock::Milliseconds64(1234);
104-
clock.mTime = k1234;
97+
clock.SetMonotonic(k1234);
10598
NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMilliseconds64() == k1234);
10699
NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMicroseconds64() == k1234);
107100

0 commit comments

Comments
 (0)