Skip to content

Replace "Future expect()" with "expectLater()". #546

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 1 commit into from
Feb 21, 2017
Merged

Conversation

nex3
Copy link
Member

@nex3 nex3 commented Feb 21, 2017

No description provided.

@nex3 nex3 requested a review from natebosch February 21, 2017 22:19
Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

LGTM

Not sure if we want to bikeshed on the name. I'm find with expectLater since I can't think of anything better (and I might have been the one that suggested it?) - but it is just a little clunky so we could ask some other folks to brainstorm. Good with me either way.

@@ -11,6 +11,12 @@ import '../backend/invoker.dart';
import '../utils.dart';
import 'async_matcher.dart';

/// A future that emits `null`.
///
/// We cache and re-use this value to avoid adding a new microtask hit for each
Copy link
Member

Choose a reason for hiding this comment

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

have we confirmed that this solves the flutter scenario?

Another pattern we could consider is to return null from _expect and always guarantee a Future from expectLater with something like => _expect(...) ?? _emptyFuture;

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we only instantiate this once, it definitely won't add more than one microtask unless it gets listeners. As long as they add something like setUpAll(() => expect(null, isNull)) it should work fine.

That said, while I'm hoping to mitigate the microtask-counting scenario, finding a surefire and highly usable solution isn't really a goal for me. The number of microtasks in the event queue at any given point is deep into "not guaranteed" territory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants