-
Notifications
You must be signed in to change notification settings - Fork 226
Take Condition for async nesting expectations #1896
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
Changes from 2 commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
28eeb2a
Take Condition for async nesting expectations
natebosch fe40a54
Rename asyncDescription
natebosch 9f25f8f
Merge branch 'master' into async-take-condition-drop-which
natebosch 8ab184b
Make `label` arguments take callbacks
natebosch 7a0724f
Expand the doc comment for Context
natebosch 10e6f47
Compare the non-nesting label case to a clause directly
natebosch bac2b3e
Mention that which can be omitted
natebosch 8529702
dart format
natebosch 4e4cf10
Enforce single string still in root ctor
natebosch 00888e6
Rename _StringClause to _ExpectationClause
natebosch 308b13d
Update docs for nest and nestAsync
natebosch e35574b
Consistently use double quotes to avoid escape single quote
natebosch 915652d
Merge commit 'e35574bb' into label-callback--context-docs
natebosch 511fa70
Merge branch 'master' into label-callback--context-docs
natebosch 911659b
Attempt to present information in a better order
natebosch 547abd3
More wording tweaks, add doc header on ConditionSubject
natebosch ef6bd7e
More wording tweaks and a paragraph on softCheck subjects
natebosch 99f9e2f
Move the 'should not throw' phrase into a template that is used in th…
natebosch 9400186
Merge branch 'master' into async-take-condition-drop-which
natebosch 5a09c33
Merge branch 'master' into async-take-condition-drop-which
natebosch 2ddbd46
Tweaks
natebosch d215a20
Merge branch 'label-callback--context-docs' into async-take-condition…
natebosch 7c9fce9
Add missing generic, use {} over =>
natebosch 3ca1aaf
Make the fields of Extracted private
natebosch db4fc6f
More working tweaks, export ConditionSubject
natebosch 7890a64
Merge branch 'label-callback--context-docs' into async-take-condition…
natebosch b33a327
Merge branch 'master' into label-callback--context-docs
natebosch 9d7fb07
Merge branch 'label-callback--context-docs' into async-take-condition…
natebosch 6b38bff
Typo
natebosch 330c3f6
Expectation extension method in Subject doc
natebosch 4d14182
missing word
natebosch 6a8e670
Link to example extensions, expand callback phrasing
natebosch afc09b3
Link the specific methods earlier
natebosch 148ef9d
Move section on callbacks being unused to much later
natebosch bae48cb
Add some examples
natebosch e09f2ae
Revert mistaken change
natebosch b9b8960
Merge branch 'master' into label-callback--context-docs
natebosch f17581f
Merge branch 'label-callback--context-docs' into async-take-condition…
natebosch bfc06ed
Merge branch 'master' into async-take-condition-drop-which
natebosch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the original here. It's explicit.
I assume that if the future has an error, the returned future has a test-failure error. (Which becomes an uncaught error if you don't do anything, which is fine, because we run tests in error zone.)
The alternative here seems to be, effectively doing a
thenfor you, sois equivalent to the original
I don't think I'd want to use a
Conditionobject directly if I can avoid it.I see three options here:
Left: Return
Future<Subject<T>>, and I assume it's a test failure if the future had an error.Right: Pass in test
Alternative: Return an "AsynchronousSubject" which accepts all the same tests as a normal
Subject, but won't apply them until the future completes. (I think I tried that, and it got very complicated very quickly.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
This also wouldn't allow for waiting for the expectation to pass before doing more work or checking more expectations. Any existing test doing
await expectLatertoday would not have a migration pattern if we lost the ability to return aFuturefor async expectations.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition is optional.
In both cases
.completes()behave identically - cause a failure if the future has en error, otherwise succeed.With the current code, chaining has multiple choices for how to write it. In order of preference for how I would write it:
After this PR, this would collapse to one choice
I like
I'm less sure that we can confidently say there are no valid use cases for choosing one of the other patterns. I'm also less sure that everyone will have the distaste for
(await check(actual).completes()).furtherCondition()that I picked up while working on this package.