Take callbacks for actual and which #1901
Draft
Conversation
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
The main benefit this brings is it brings more alignment with the `clause` arguments for `expect` calls. The docs will be able to focus on the difference in how the value is use (preceding "that" in the case of labels, standing on its own in a list in the case of clauses) and can use a consistent description for how it is passed. A secondary benefit is that it allows multiline labels and avoid workaround like joining with `r'\n'`. A final benefit is that it saves some unnecessary String formatting since the callback isn't called if no expectations fail on the Subject, or when used as a soft check where the failure details are ignored. - Make the `label` arguments to `nest` and `nestAsync`, and the _label field in `_TestContext` an `Iterable<String> Function()`. - Wrap strings that had been passed to `String` arguments with callbacks that return the string in a list. - When writing the label in a failure, write all lines, and use a postfix " that:". - Update some `Map` expectations which had manually joined with literal slash-n to keep the label or clause to a single line to take advantage of the multiline allowance. Split tests for the changed implementations and add tests for the descriptions with multiline examples. Some of these could have used multiline clauses before.
natebosch
force-pushed
the
label-callback--lazy-rejection-strings
branch
from
1079df0 to
16ebb0b
Compare
This aligns these arguments with all the other failure formatting arguments like `clause` and `label`. There may be some performance benefit in some cases where a rejection from `softCheck` is ignored and expensive String operations are avoid, but in the typical case this just introduces closures which will be invoked shortly. Replace the default empty list for `actual` with a default function. This is a slight behavior change where passing `() => []` will not get overwritten with the default, but passing `[]` would have. Only defaulting for when the argument was not passed at all is slightly better behavior. Replace a bunch of `Iterable<String>` with `Iterable<String> Function()` and invoke them at the moment the strings are needed.
natebosch
force-pushed
the
label-callback--lazy-rejection-strings
branch
from
16ebb0b to
b95c060
Compare
natebosch
commented
Feb 3, 2023
| 'was accepted by the condition checking:', | ||
| ...await describeAsync(condition) | ||
| ]); | ||
| final description = await describeAsync(condition); |
There was a problem hiding this comment.
Here is one place where we are not getting value from the callback potentially not getting invoked.
I can't make the which callback async, so we are forced to do this async work outside of the callback.
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.
This aligns these arguments with all the other failure formatting
arguments like
clauseandlabel. There may be some performancebenefit in some cases where a rejection from
softCheckis ignored andexpensive String operations are avoid, but in the typical case this just
introduces closures which will be invoked shortly.
Replace the default empty list for
actualwith a default function.This is a slight behavior change where passing
() => []will not getoverwritten with the default, but passing
[]would have. Onlydefaulting for when the argument was not passed at all is slightly
better behavior.
Replace a bunch of
Iterable<String>withIterable<String> Function()and invoke them at the moment the strings are needed.