Skip to content
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

Use clock package in _TimeIntervalStreamSink (and maybe others) #660

Open
nightscape opened this issue Feb 1, 2022 · 7 comments
Open

Use clock package in _TimeIntervalStreamSink (and maybe others) #660

nightscape opened this issue Feb 1, 2022 · 7 comments

Comments

@nightscape
Copy link

In order to deterministically test async code, the fakeAsync package currently seems to be the way to go.
Unfortunately, it cannot modify the StopWatch from dart:core, and instead it is recommended to use the clock package which describes itself as

This package provides a Clock class which encapsulates the notion of the "current time" and provides easy access to points relative to the current time. Different Clocks can have a different notion of the current time, and the default top-level clock's notion can be swapped out to reliably test timing-dependent code.

Would it be possible to use this package in _TimeIntervalStreamSink and possibly other places, in order to allow deterministic testing using fakeAsync?

I'd be happy to contribute a corresponding PR!

@hoc081098
Copy link
Collaborator

Sound reasonable!
I have just opened PR #661. cc @frankpepermans, please take a look at it :))

@nightscape
Copy link
Author

@hoc081098 wow, that was quick!!
Thanks a lot ❤️
I'm already trying this out locally, will let you know how it works.

@brianegan
Copy link
Collaborator

As a heads up, we intentionally removed fakeAsync tests from this library because it is fundamentally broken for Streams & timers. FakeAsync can only modify the current Zone's async timers, but the Stream library often runs code in the root Zone (outside the control of FakeAsync).

Therefore, I'd be careful with this change, since it might give folks the impression that it's safe to use FakeAsync, but the fundamental issues are still unsolved.

Please see these discussions:

@nightscape
Copy link
Author

Oh, very good point! I wasn't aware of this at all.
I skimmed the discussions and the issue seems to be a rather fundamental one,
where it's not clear if the current behaviour will ever be changed.
Not sure what the best approach would be here, kind of a "damned if you do, damned if you don't" situation...

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Jun 8, 2022

+1 This is very much needed, but the FakeAsync's problem seems also big...

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Jun 8, 2022

However, I see the flutter repo itself uses fake_async a lot. Indeed testWidgets by default uses it. How did they solve the problem?

@MiniSuperDev
Copy link

Hello, any update on this?

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

No branches or pull requests

5 participants