Skip to content

Migrate away from fake_time #8782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2025
Merged

Conversation

jonasfj
Copy link
Member

@jonasfj jonasfj commented May 22, 2025

Instead we'll have:

  • clockControl which can be used to elapse time within testWithProfile
  • Whenever you want a delay that is affected by clockControl you can use:
    • Future.timeoutWithClock in place of Future.timeout
    • clock.delayed in place of Future.delayed

This way, a timeout or delay that is used to handle an I/O operation that doesn't complete and use Future.timeout (or Future.delayed).

While a delay or timeout that is used to schedule something after a certain delay can used clock.delayed (or Future.timeoutWithClock).

One of these can be artificially accelerated with clockControl other can't.

If you accelerate a 300ms timeout that throws an error if an I/O operation doesn't complete in 300ms, then it's hard to write a test where the I/O operation succeeds.

If you don't accelerate a delay where you're waiting an hour before rescheduling a task, then tests will take forever to run.

Hence, we have normal timeout / Future.delayed, and we have clock.delayed and Future.timeoutWithClock, which are controlled with clockControl.

@jonasfj jonasfj requested a review from isoos May 22, 2025 11:56
@jonasfj jonasfj force-pushed the deprecated-fake-async branch from f358825 to 43baf12 Compare May 22, 2025 12:02
Copy link
Collaborator

@isoos isoos left a comment

Choose a reason for hiding this comment

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

The new functions seem good to me, however the naming could be more consistent.

How about:

  • Future.timeoutWithClock -> Future.controllableTimeout, because (a) there is no clock provided to the method, and (b) the control of the timeout is outside of the arguments.
  • Clock.delayed -> top-level delayedFuture(Duration delay) method, as there is very little need extend it on the clock object, is there? We are not using the instance of it.

@jonasfj
Copy link
Member Author

jonasfj commented May 22, 2025

  • Future.timeoutWithClock -> Future.controllableTimeout, because (a) there is no clock provided to the method, and (b) the control of the timeout is outside of the arguments.
  • Clock.delayed -> top-level delayedFuture(Duration delay) method, as there is very little need extend it on the clock object, is there? We are not using the instance of it.

I could be down for using controllableDelay and controllableTimeout.

But clockControl also changes clock, so I figured it might be fine to treat it as an extension of clock.

Logically, I think we want ClockController.elapse to advance these futures and the clock at the same time, which is why I figured clock.delayed was a good term.

I'm not sure I like Future.timeoutWithClock, but we can always rename and refactor.

@jonasfj jonasfj requested a review from isoos May 22, 2025 13:08
@jonasfj jonasfj merged commit 1eb1135 into dart-lang:master May 22, 2025
31 checks passed
@jonasfj jonasfj deleted the deprecated-fake-async branch May 22, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants