Skip to content

feat(dart): inject clocks into GenUI timestamps and timeouts (#2239) - #2265

Open
Varun-S10 wants to merge 2 commits into
a2ui-project:mainfrom
Varun-S10:feat/issue-2239
Open

feat(dart): inject clocks into GenUI timestamps and timeouts (#2239)#2265
Varun-S10 wants to merge 2 commits into
a2ui-project:mainfrom
Varun-S10:feat/issue-2239

Conversation

@Varun-S10

Copy link
Copy Markdown
Collaborator

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 hardcoded DateTime.now().
  • SurfaceGroupModel & MessageProcessor: Passes the injected clock down to newly created surfaces.
  • CancellationSignal.timeout: Allows testing function timeouts using fake timers.
  • Backwards Compatibility: All new clock parameters are optional and default to normal system time. Existing code is not affected.
  • Unit Tests: Added tests for FakeClock, SurfaceModel, MessageProcessor, and CancellationSignal.

Pre-launch Checklist

One time:

For this PR:

  • I have updated the relevant CHANGELOG.md file.
  • I updated/added relevant documentation.
  • My code changes (if any) have tests.
  • If my branch is on a fork, I have verified that scripts/e2e_test.sh passes.

If you need help, consider asking for advice on the discussion board.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines 31 to +37
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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied the changes as suggested and added a unit test to verify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE]: Inject clocks into Dart/Flutter GenUI timestamps and timeouts

1 participant