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

allow Matcher.matches to return Future<bool> #211

Closed
DartBot opened this issue Jun 5, 2015 · 7 comments
Closed

allow Matcher.matches to return Future<bool> #211

DartBot opened this issue Jun 5, 2015 · 7 comments
Labels
closed-not-planned Closed as we don't intend to take action on the reported issue type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/444270?v=3" align="left" width="96" height="96"hspace="10"> Issue by seaneagan
Originally opened as dart-lang/sdk#10188


Some tests cannot be completed synchronously, in which case it would make sense to return a Future<bool> as opposed to bool from Matcher.matches.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2156198?v=3" align="left" width="48" height="48"hspace="10"> Comment by kasperl


Added Area-UnitTest, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2762632?v=3" align="left" width="48" height="48"hspace="10"> Comment by gramster


I don't like the idea of changing matches(); there are other ways to achieve this. In particular, you can return a Future from the test function.

@DartBot DartBot added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) Priority-Medium labels Jun 5, 2015
@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/444270?v=3" align="left" width="48" height="48"hspace="10"> Comment by seaneagan


So the goal here is to allow users to define their own reusable and
composable asynchronous matchers, so it would need to be implemented in the
Matcher itself. See for example issue dart-lang/matcher#13. Another example would be
matchers for http requests to test a REST API:

test(someRestEndPoint, allOf(hasStatus(HttpStatus.OK),
hasContentType('application/json')));

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2762632?v=3" align="left" width="48" height="48"hspace="10"> Comment by gramster


What I'm suggesting is you could do this as:

test('', () {
  return makeRestRequest(...).then((result) {
     expect(result, allOf(hasContentType('...'), ...));
  });
});

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/2762632?v=3" align="left" width="48" height="48"hspace="10"> Comment by gramster


Added NotPlanned label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/17034?v=3" align="left" width="48" height="48"hspace="10"> Comment by kevmoo


Added Pkg-Unittest label.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/17034?v=3" align="left" width="48" height="48"hspace="10"> Comment by kevmoo


Removed Area-UnitTest label.
Added Area-Pkg label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-not-planned Closed as we don't intend to take action on the reported issue type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

1 participant