feat(dart): inject clocks into GenUI timestamps and timeouts (#2239) - #2265
feat(dart): inject clocks into GenUI timestamps and timeouts (#2239)#2265Varun-S10 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a clock injection feature to enable deterministic testing and timing control across the A2UI system, adding a Clock typedef, a FakeClock class, and integrating clock propagation into SurfaceModel, SurfaceGroupModel, and MessageProcessor. It also adds a CancellationSignal.timeout factory constructor with an injectable timer factory. The reviewer identified a potential synchronization issue in MessageProcessor where an omitted clock parameter could mismatch with an explicitly provided groupModel's clock, and suggested defaulting the clock to groupModel.clock when available.
| MessageProcessor({ | ||
| required this.catalogs, | ||
| void Function(A2uiClientAction)? onAction, | ||
| }) : groupModel = SurfaceGroupModel<T>() { | ||
| Clock? clock, | ||
| SurfaceGroupModel<T>? groupModel, | ||
| }) : clock = clock ?? systemClock, | ||
| groupModel = groupModel ?? SurfaceGroupModel<T>(clock: clock) { |
There was a problem hiding this comment.
If a custom groupModel is passed to the MessageProcessor constructor but clock is omitted, this.clock will default to systemClock while groupModel might be using a different clock (such as a FakeClock in tests). This can cause dynamically created surfaces to use a different clock than the rest of the group, leading to inconsistent timestamps and hard-to-debug timing issues.
We should default clock to groupModel.clock if groupModel is provided, to ensure they stay in sync.
MessageProcessor({
required this.catalogs,
void Function(A2uiClientAction)? onAction,
Clock? clock,
SurfaceGroupModel<T>? groupModel,
}) : clock = clock ?? groupModel?.clock ?? systemClock,
groupModel = groupModel ?? SurfaceGroupModel<T>(clock: clock) {There was a problem hiding this comment.
Applied the changes as suggested and added a unit test to verify.
Fixes #2239
Description
This PR adds an optional clock and timer injection to
dart/a2ui_core.Previously, action timestamps used hardcoded real-time
DateTime.now(), which made tests hard to predict and slow when testing timeouts. With this change, tests can pass a fake clock to control time instantly without sleeping.What is added:
Clock&FakeClock(clock.dart): Added helper classes to control time in tests.SurfaceModel: Now uses the injected clock for action timestamps instead of hardcodedDateTime.now().SurfaceGroupModel&MessageProcessor: Passes the injected clock down to newly created surfaces.CancellationSignal.timeout: Allows testing function timeouts using fake timers.FakeClock,SurfaceModel,MessageProcessor, andCancellationSignal.Pre-launch Checklist
One time:
For this PR:
If you need help, consider asking for advice on the discussion board.