Throw when an async matcher is used with isNot#2663
Draft
natebosch wants to merge 4 commits into
Draft
Conversation
Closes #1637 Async matchers are implemented as a hack where they usually appear to succeed synchronously, and may report an error for the test case later. This is incompatible with meta matchers or "operator" matchers `isNot` and `anyOf` where the synchronous answer is trusted and other values are derived from it. In the case of `isNot` this causes false positives, and for `anyOf` it causes false negatives. Historically it was infeasible to add knowledg about `AsyncMatcher` in the definition of these operators, because async matching was implemented in the test runner directly and the matchers exposed by the test runner are a superset of those defined by the matcher package. Now that `expect` and the async matchers are definied in `package:matcher` it is possible to detect when an async matcher is used this way. There is one async matcher when does often serve as a synchronous matcher in practice - the `throwsA` matcher is used for both synchronous and asynchronous errors, the same matcher is uesed for `Function()`, `Future Function()` and `Future` values. I think the combination of `throwsA` and `isNot` or `anyOf` is likely to be rare, but if errors surface in practice we may need to add an exception for the `Throws` matcher.
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with |
Member
Author
|
This is still a tricky import to add internally because |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1637
Async matchers are implemented as a hack where they usually appear to
succeed synchronously, and may report an error for the test case later.
This is incompatible with meta matchers or "operator" matchers
isNotand
anyOfwhere the synchronous answer is trusted and other values arederived from it. In the case of
isNotthis causes false positives, andfor
anyOfit causes false negatives.Historically it was infeasible to add knowledg about
AsyncMatcherinthe definition of these operators, because async matching was
implemented in the test runner directly and the matchers exposed by the
test runner are a superset of those defined by the matcher package. Now
that
expectand the async matchers are definied inpackage:matcherit is possible to detect when an async matcher is used this way.
There is one async matcher when does often serve as a synchronous
matcher in practice - the
throwsAmatcher is used for both synchronousand asynchronous errors, the same matcher is uesed for
Function(),Future Function()andFuturevalues. I think the combination ofthrowsAandisNotoranyOfis likely to be rare, but if errorssurface in practice we may need to add an exception for the
Throwsmatcher.