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 FIFO timer ordering in FakeAsync (fixes #258) #265

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@alechenninger
Contributor

alechenninger commented Aug 13, 2015

As noted in #258 , the current ordering of _getNextTimer() in _FakeAsync is inconsistent with the native Dart event loop.

In the process of fixing that, I had to fix a few other idiosyncrasies that were revealed from the existing unit tests which relied on the incorrect behavior. For instance, if you had a periodic timer occur at the same time as a delayed timer, whether or not both of them were "flushed" in flushTimers depended on the order with which they were queued. Now it does not.

I tried to stick to the project's existing conventions. Thanks for any feedback!

@alechenninger

This comment has been minimized.

Show comment
Hide comment
@alechenninger

alechenninger Oct 31, 2015

Contributor

Any thoughts on this? Thanks!

Contributor

alechenninger commented Oct 31, 2015

Any thoughts on this? Thanks!

@yjbanov

This comment has been minimized.

Show comment
Hide comment
@yjbanov

yjbanov Jan 27, 2016

Member

LGTM. Could you send a similar fix to https://github.com/QuiverDart/quiver_async?

Member

yjbanov commented Jan 27, 2016

LGTM. Could you send a similar fix to https://github.com/QuiverDart/quiver_async?

@yjbanov yjbanov added the lgtm label Jan 27, 2016

@alechenninger

This comment has been minimized.

Show comment
Hide comment
@alechenninger

alechenninger Jan 29, 2016

Contributor

Sure! Thanks for taking a look. I'll open a PR this weekend.

Contributor

alechenninger commented Jan 29, 2016

Sure! Thanks for taking a look. I'll open a PR this weekend.

cbracken added a commit that referenced this pull request Nov 16, 2017

Use FIFO timer ordering in FakeAsync (#265)
As noted in #258, the
current ordering of _getNextTimer() in _FakeAsync is inconsistent with
the native Dart event loop.

In the process of fixing that, I had to fix a few other idiosyncrasies
that were revealed from the existing unit tests which relied on the
incorrect behavior. For instance, if you had a periodic timer occur at
the same time as a delayed timer, whether or not both of them were
"flushed" in flushTimers depended on the order with which they were
queued. Now it does not.
@cbracken

This comment has been minimized.

Show comment
Hide comment
@cbracken

cbracken Nov 16, 2017

Member

Manually merged as ccf476c.

Apologies for the delay in getting this landed here. Was just doing some housecleaning and noticed this PR was merged downstream but never here. Merged now. Please feel free to send a PR adding yourself to the authors list in pubspec.yaml :)

Member

cbracken commented Nov 16, 2017

Manually merged as ccf476c.

Apologies for the delay in getting this landed here. Was just doing some housecleaning and noticed this PR was merged downstream but never here. Merged now. Please feel free to send a PR adding yourself to the authors list in pubspec.yaml :)

@cbracken cbracken closed this Nov 16, 2017

@alechenninger

This comment has been minimized.

Show comment
Hide comment
@alechenninger

alechenninger Nov 17, 2017

Contributor

Thanks!

Contributor

alechenninger commented Nov 17, 2017

Thanks!

alechenninger added a commit to alechenninger/quiver-dart that referenced this pull request Nov 17, 2017

cbracken added a commit that referenced this pull request Nov 17, 2017

Add Alec Henninger to authors
Patches landed to FakeAsync in #265
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment