Repository navigation
[checks] Add CheckFailure.describeNested - #2793
Conversation
Expectations which soft check a condition against part of their value, such as `IterableChecks.every`, describe the failure within the `which` of their own rejection. Each of `every`, `StreamChecks.inOrder`, `StreamChecks.anyOf`, and `IterableChecks.pairwiseMatches` formatted the nested failure by hand, each slightly differently, and each dropped detail in some cases. Two bugs in `every` were reported in #2784 (review). Add `CheckFailure.describeNested`, which describes a failure in the same format as a failure message from `check`, without the line for the label of the root subject, since a soft checked condition has no root label. The caller provides its own line in its place, such as `has an element at index 0 that:`. `check` and `describeNested` share a private method which produces the "Actual:" and "Which:" lines, so the two formats agree by construction. `CheckFailure` is exported only from `package:checks/context.dart`, so the method is available to authors of expectations without being part of the API for writing checks. Use `describeNested` from `every`, `inOrder`, `anyOf`, and `pairwiseMatches`. - `every` read `failure.detail.actual.skip(1)` followed by the rejected value, which lost the property name when the nested failure collapsed: Which: has an element at index 0 that: Actual: 'foo' It now reads `Actual: has field: 'foo'`. - `inOrder` and `anyOf` dropped the actual description entirely when a condition failed without collapsing, including when the actual collapsed to the root. They now always include an `Actual:` line, and no longer repeat an `Expected:` line for a collapsed failure since the outer description already lists each condition. - `pairwiseMatches` keeps the description of the condition for the element, followed by `Actual:` in place of `Actual element at index N:`. Whether the actual description still starts with the root label line is recorded on `FailureDetail` where the description is built. It is false when the actual collapses onto a single line which replaces the label line. Fix the failure details when a soft check rejects a precondition such as `isA<T>()` and further expectations follow it, which also affected `every`: check([Wrapper(123)]) .every(.it()..value.isA<Foo>().field.equals('bar')); Failed assertion: 'foundDepth == -1': is not true. `_fullDetails` assumed that when a subject itself failed, the failing clause was the last one added to it. That holds for `check`, where a failure throws before anything else is added, but not for a soft check, where the following nested clauses are still added, nor for an unawaited expectation, which can fail after later clauses are added. Pass the rejected clause from each failure through to `_fullDetails` instead of relying on `_clauses.last`. A rejected guard is also no longer added a second time as a `_GuardClause`, since the failing expectation clause already describes it, so the precondition appears once in the description.
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 |
| 'failed to satisfy the condition at index 1', | ||
| 'because it closed without emitting enough values', | ||
| 'because it:', | ||
| ' Actual: a stream', |
There was a problem hiding this comment.
this is a slight regression in some cases because it remove some special case handling when it migrated to a shared code path. This formulation of the output isn't unique to this PR so I think we should land it even with some test cases looking a bit worse. In a subsequent update I may take a look at outputting something more specific than "a stream".
There was a problem hiding this comment.
Following up in #2811 to put a description of what the stream did in the "Actual:" part.
davidmorgan
left a comment
There was a problem hiding this comment.
The failingClause threading fixes the foundDepth == -1 assertion, and describeNested is a good consolidation. Soft checks with more than one clause still produce misleading output, though, and deepEquals wasn't migrated.
- In a soft check, clauses after the failing one are still added, and
_fullDetailsappends their expected descriptions toactual. They read as if the actual value satisfied them, andWhich:ends up attached to the last line:
check([Foo('foo')]).every(.it()
..field.startsWith('f')
..field.equals('bar')
..field.length.equals(999));
Which: has an element at index 0 that:
has field: a string starting with 'f'
has field: 'foo'
has field that has length: <999>
Which: differs at offset 0:
If the failing clause comes first, the actual also collapses onto the root line, and the later clauses print as its children (see the inline comment). The new tests only cover passing clauses before the failing one.
-
collection_equality.dart_compareValuestill formats a soft-checkedConditionfailure by hand.check([Foo('foo')]).deepEquals([Condition.it<Object?>()..isA<Foo>().field.equals('bar')])still printsActual: 'foo'without the property, which is theeverybug this PR fixes. -
Minor: the public
FailureDetailconstructor hard-codes_actualHasLabel = true, sodescribeNesteddrops the first line of a customactualbuilt through it.
# Conflicts: # pkgs/checks/CHANGELOG.md
A failure from `check` throws as soon as an expectation is rejected, so the
failure details only describe the expectations up to the failing one. A
soft check keeps going after a rejection, and an unawaited expectation can
fail after later expectations were made, so in those cases the later clauses
were already added to the subject. `_fullDetails` described them along with
the actual value, as if the value satisfied them:
check([Foo('foo')]).every(.it()
..field.startsWith('f')
..field.equals('bar')
..field.length.equals(999));
Which: has an element at index 0 that:
has field: a string starting with 'f'
has field: 'foo'
has field that has length: <999>
Which: differs at offset 0:
When the failing clause came first, the later clauses also printed as
children of the actual which had collapsed onto one line:
Actual: has value: <123>
has value: <5>
Which: is not equal
Only describe the clauses up to and including the one on the path to the
failure, both when collapsing and when describing the full structure. The
examples above now end at `has field: 'foo'`, and collapse to
`Actual: has value: <123>`.
An asynchronous nested expectation, such as `completes`, only adds its
clause once its value is available. An expectation made after it on the
same subject, which runs while it waits, is added first, so it is still
described, ahead of the failing clause.
An asynchronous nested expectation, such as `completes`, only added its
clause once its value was available. An expectation made after it on the
same subject runs while it waits, so its clause was added first. When the
asynchronous expectation then failed, the later expectation was described
ahead of it, and since it came first in the list it was kept by the change
to omit clauses after the failure:
final subject = check(Future.value(1));
final completion = subject.completes(.it()..equals(2));
subject.has((f) => 0, 'zero').equals(0);
await completion;
Expected: a Future<int> that:
has zero: <0>
completes to <2>
Add a placeholder clause when the asynchronous expectation is made, and
replace it with the nested subject, or with the failing expectation, once
the value is available. Expectations made in the meantime come after it, so
they are omitted when it fails, the same as a failure from a synchronous
expectation:
Expected: a Future<int> that completes to <2>
Actual: a Future<int> that completes to <1>
Which: is not equal
A placeholder is not described, so a later expectation which fails while
the asynchronous expectation is still waiting is described without it, as
before.
An expectation from `Context.expectAsync` added its clause as soon as it
was made, before its predicate had finished. When a later nested
expectation failed while the predicate was still running, the unfinished
expectation was listed under `Actual:` as if the value satisfied it.
`expectAsync` now adds a `_PendingClause` placeholder, the same one used
by `nestAsync`, and replaces it with the expectation once the predicate
finishes. The placeholder keeps the place of the expectation in the order
the expectations were made, and is never described, so a failure before
the predicate finishes leaves the unfinished expectation out. For example
with an expectation from `expectAsync` that has not finished followed by
`has((v) => v, 'self').equals(1)` on `0`, the failure now reads:
Expected: a int that has self: <1>
Actual: a int that has self: <0>
Which: is not equal
Expectations from `Context.expectUnawaited`, such as `doesNotComplete`,
are left as they are. They are never awaited by the check, so they are
described as satisfied until they fail.
When an element of the expected collection is a `Condition` and the
actual element fails it, `deepEquals` formatted the failure by hand from
the `FailureDetail` and `Rejection`. That output lost the property the
rejection was for when the failure collapsed, repeated the value that
was already described when it didn't, and used `which` in place of
`Which:`.
Use `CheckFailure.describeNested`, the same as `IterableChecks.every`.
For example, with `['abc']` against
`[Condition.it<dynamic>()..isA<String>().length.equals(2)]`, the which
was:
has an element at [<0>] that:
Actual: <3>
which is not equal
and is now:
has an element at [<0>] that:
Actual: has length: <3>
Which: is not equal
When the condition doesn't collapse, the `Actual: <3>` line that repeated
`has length: <3>` is gone. The deeper indentation and the missing
`Actual:` prefix in that form are the same as for `every`, and are left
for a separate change.
When a soft checked failure doesn't collapse to a single line,
`describeNested` dropped the line for the root label, which was the only
line with `Actual:`. The remaining lines were indented 4 spaces under
the caller's line, where a collapsed failure is indented 2, and nothing
marked where the description of the actual value started. In
`pairwiseMatches`, which describes the condition first, the expected and
actual descriptions ran together:
does not have an element at index 0 that:
has field: 'foo'
has field that has length: <4>
has field: 'foo'
has field that has length: <3>
Which: is not equal
Replace the root label line with a line containing only `Actual:`, in
the place of the `Actual: <label> that:` line in a failure message from
`check`:
does not have an element at index 0 that:
has field: 'foo'
has field that has length: <4>
Actual:
has field: 'foo'
has field that has length: <3>
Which: is not equal
Every nested failure now has an `Actual:` line, as the CHANGELOG already
says.
`FailureDetail` had a public constructor and a private one. The public
constructor recorded that a custom `actual` started with the line for the
root label, so `describeNested` dropped the first line of an `actual`
passed to it:
final failure = CheckFailure(
Rejection(actual: ['<3>'], which: ['is more than 2']),
() => FailureDetail(
['a Box that:', ' has size: less than <2>'],
1,
0,
actual: ['has size: <3>'],
),
);
described the failure as only `Which: is more than 2`.
Nothing outside the package needs to construct either class. `Condition`
and `Context` are `abstract final`, so a failure only comes from soft
checking a `Condition`, and no API accepts a `CheckFailure` or
`FailureDetail` as an argument. All uses of the constructors are in
`checks.dart`.
Make the `CheckFailure` constructor private, and merge the two
`FailureDetail` constructors into one private constructor which takes
`collapsedDetails`, `actual`, and `actualHasLabel`. The classes are still
exported from `package:checks/context.dart` so a failure can be read.
These should be hidden now.
Good catch. Migrated to the new utility.
I think it was an oversight that these were ever public in the first place. Dropping the public constructors. |
Revisions updated by `dart tools/rev_sdk_deps.dart`. core (https://github.com/dart-lang/core/compare/d7bd45c..24e09c4): 24e09c4c Sun Nov 1 21:12:19 2026 +0000 dependabot[bot] Bump the github-actions group with 4 updates (dart-lang/core#1037) 87ac1b4c Fri Oct 30 16:02:43 2026 +0100 Michael Thomsen [os_detect] Deprecate package:os_detect in favor of package:platform (dart-lang/core#1003) 8debf335 Fri Oct 30 01:57:36 2026 +0530 Sahaja Pallapothula Clarify Characters equality and Unicode normalization (dart-lang/core#1031) ecosystem (https://github.com/dart-lang/ecosystem/compare/24b9ba8..f8faabf): f8faabf Sun Nov 1 18:16:20 2026 +0000 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/ecosystem#481) 27c2fc8 Sat Oct 31 12:36:53 2026 +0100 Moritz Fix publish workflow pins, add tag_prefix to health, and update docs (dart-lang/ecosystem#480) 05641a9 Sat Oct 31 09:56:42 2026 +0100 Moritz Restore deprecated publish.yaml workflow and firehose CLI entrypoint (dart-lang/ecosystem#479) 78abf72 Fri Oct 30 08:24:07 2026 -0700 Kevin Moore fix(repo_manage): use pub_semver for changelog -wip transitions (dart-lang/ecosystem#475) 0bfa654 Fri Oct 30 09:33:57 2026 +0100 Moritz Move publish check to health workflow and use individual package publishing workflows from `setup-dart` (dart-lang/ecosystem#454) adf7979 Thu Oct 29 23:46:49 2026 -0700 Kevin Moore test(firehose): use hasLength, containsPair, and isFalse (dart-lang/ecosystem#477) http (https://github.com/dart-lang/http/compare/c76dfb3..8c0d66a): 8c0d66a Fri Oct 30 09:54:41 2026 -0700 Brian Quinlan chore(cupertino_http): check ffigen generation (dart-lang/http#2044) c8737d1 Fri Oct 30 09:11:46 2026 +0100 Moritz chore(http): run Firefox tests with -headless instead of xvfb-run (dart-lang/http#2033) 05d0b18 Fri Oct 30 07:08:19 2026 +0100 Moritz fix(http2): make StreamException a StreamTransportException and forward ALPN errors to onError (dart-lang/http#2009) i18n (https://github.com/dart-lang/i18n/compare/4c7b4ad..7475ce6): 7475ce64 Sat Oct 31 02:49:28 2026 +0100 Moritz [intl4x] Fix NumberFormat.percent ignoring digits (dart-lang/i18n#1115) shelf (https://github.com/dart-lang/shelf/compare/62d3e4e..9946de5): 9946de5 Sun Nov 1 12:27:59 2026 +0000 dependabot[bot] build(deps): bump the github-actions group with 3 updates (dart-lang/shelf#554) test (https://github.com/dart-lang/test/compare/6d74720..2a9e017): 2a9e0175 Sun Nov 1 03:47:24 2026 +0000 dependabot[bot] Bump the github-actions group with 6 updates (dart-lang/test#2813) 55cd5cd5 Fri Oct 30 13:43:47 2026 -0700 Nate Bosch Migrate publishing workflows from firehose to setup-dart (dart-lang/test#2812) 10dc78c0 Fri Oct 30 11:21:53 2026 -0700 Nate Bosch [checks] Describe what the stream did in failures (dart-lang/test#2811) c11ad06d Fri Oct 30 08:59:57 2026 -0700 Nate Bosch [checks] Add CheckFailure.describeNested (dart-lang/test#2793) 0137b43e Fri Oct 30 05:40:43 2026 +0530 Shriraj Jadhav [checks] Use the Iterable.last member for the last expectation (dart-lang/test#2807) 50c9f454 Thu Oct 29 15:11:39 2026 -0700 Nate Bosch [checks] Collapse clauses for described conditions (dart-lang/test#2794) tools (https://github.com/dart-lang/tools/compare/3980dca..1579f8d): 1579f8d69 Mon Nov 2 02:44:58 2026 +0530 Joshua fix(coverage): reject package: and dart: URIs outside known roots in Resolver (dart-lang/tools#2764) f869fdca4 Sun Nov 1 10:26:34 2026 +0100 dart-ecosystem-bot[bot] [boring] Update BoringSSL to ed9209cf (dart-lang/tools#2774) d10e2b87f Sun Nov 1 10:26:11 2026 +0100 dart-ecosystem-bot[bot] [boring] Pin the prebuilt binaries of boring-binaries-20261101-33e9e69722af (dart-lang/tools#2777) 33e9e6972 Sun Nov 1 05:56:30 2026 +0000 dependabot[bot] build(deps): bump the github-actions group with 4 updates (dart-lang/tools#2775) f84d0d0d2 Sun Nov 1 05:36:12 2026 +0000 dependabot[bot] build(deps): bump ffigen from 22.0.0 to 23.0.0 in /pkgs/boring (dart-lang/tools#2776) 14ad5e515 Fri Oct 30 10:48:58 2026 -0700 Kevin Moore [mime] Harden BoundMultipartStream error routing, headers, and epilogue (dart-lang/tools#2629) 7a780d48c Fri Oct 30 12:57:03 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid rescanning failed plain-text runs in InlineParser (dart-lang/tools#2757) 3671d33a9 Fri Oct 30 12:56:48 2026 +0100 Jonas Finnemann Jensen [markdown] Index footnote definitions and labels on Document for O(1) lookup (dart-lang/tools#2760) 6f38e9fda Fri Oct 30 12:56:09 2026 +0100 Jonas Finnemann Jensen [markdown] Bound the domain of extended URL autolinks to DNS limits (dart-lang/tools#2759) aa0c38544 Fri Oct 30 12:20:40 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid quadratic removeRange in _combineAdjacentText (dart-lang/tools#2758) f425baf46 Fri Oct 30 11:16:28 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid rescanning failed space runs in LineBreakSyntax (dart-lang/tools#2756) c7eafbec3 Fri Oct 30 11:09:45 2026 +0100 Jonas Finnemann Jensen [markdown] Reject '<' in AutolinkSyntax (dart-lang/tools#2755) 86ed0f902 Fri Oct 30 11:08:25 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid quadratic backtracking in headerPattern (dart-lang/tools#2754) 8ea07517f Fri Oct 30 07:45:42 2026 +0100 Moritz [boring] Regenerate the x509-limbo expected failures in the refresh job (dart-lang/tools#2712) 40235c365 Thu Oct 29 14:46:17 2026 +0100 Jonas Finnemann Jensen [markdown] Make HtmlBlockSyntax.parseChildLines iterative (dart-lang/tools#2749) 0ff03f643 Thu Oct 29 14:41:33 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid re-escaping nested image alt text in ImageSyntax.createNode (dart-lang/tools#2752) 784654d01 Thu Oct 29 14:37:11 2026 +0100 Jonas Finnemann Jensen [markdown] Add test cases covering parser and renderer edge cases (dart-lang/tools#2753) 4f2718ad8 Thu Oct 29 14:36:52 2026 +0100 Jonas Finnemann Jensen [markdown] Make Document._parseInlineContent iterative (dart-lang/tools#2750) cac1421d5 Thu Oct 29 14:17:06 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid quadratic duplicate heading ID probing in HtmlRenderer (dart-lang/tools#2748) 66880bcd3 Thu Oct 29 14:11:27 2026 +0100 Jonas Finnemann Jensen [markdown] Check footnote label and empty map before scanning footnote keys (dart-lang/tools#2751) 7f8696523 Thu Oct 29 14:07:06 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid quadratic blank-line lookahead in CodeBlockSyntax (dart-lang/tools#2747) 9da6bb291 Thu Oct 29 14:03:04 2026 +0100 Jonas Finnemann Jensen [markdown] Avoid exponential and quadratic backtracking in tablePattern (dart-lang/tools#2746) dartdoc is held back at db94437, before dart-lang/dartdoc#4312, because its mock SDK version changes conflict in the google3 3-way merge with the hotfix in cl/1005963642. native is held back at e0ca7c6, before dart-lang/native#3742, because deprecating `MacOSCodeConfig(targetVersion: ...)` causes a fatal `deprecated_member_use` diagnostic in `pkg/dartdev/lib/src/native_assets.dart`. web is held back at 4e60e61, before dart-lang/web#596, because it changes `package:web` signatures (e.g. `innerHTML=`, `parseFromString`, `Worker()`) from `JSAny` to `String` in ways that break callers in google3. Change-Id: I53d2cbde741c45b8fd33bca42742549819dd332b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/572780 Reviewed-by: Brian Quinlan <[email protected]> Commit-Queue: Nate Bosch <[email protected]>
Expectations which soft check a condition against part of their value,
such as
IterableChecks.every, describe the failure within thewhichoftheir own rejection. Each of
every,IterableChecks.deepEquals,StreamChecks.inOrder,StreamChecks.anyOf, andIterableChecks.pairwiseMatchesformatted the nested failure by hand, eachslightly differently, and each dropped detail in some cases. Two bugs in
everywere reported in#2784 (review).
Add
CheckFailure.describeNested, which describes a failure in the sameformat as a failure message from
check. A soft checked condition has nolabel for its root subject, so the caller provides its own line describing
the value, such as
has an element at index 0 that:. When the actual doesnot collapse to a single line, a line with only
Actual:takes the placeof the
Actual: <label> that:line fromcheck.checkanddescribeNestedshare a private method which produces the "Actual:" and"Which:" lines, so the two formats agree by construction.
CheckFailureisexported only from
package:checks/context.dart, so the method isavailable to authors of expectations without being part of the API for
writing checks.
Use
describeNestedfromevery,deepEquals,inOrder,anyOf, andpairwiseMatches.everyanddeepEqualslost the property name when the nested failurecollapsed:
It now reads
Actual: has field: 'foo'.deepEqualsalso usesWhich:in place of
which.inOrderandanyOfdropped the actual description entirely when acondition failed without collapsing, including when the actual collapsed
to the root. They now always include an
Actual:line, and no longerrepeat an
Expected:line for a collapsed failure since the outerdescription already lists each condition.
pairwiseMatcheskeeps the description of the condition for the element,followed by
Actual:in place ofActual element at index N:, so theexpected and actual descriptions are separated:
Fix the failure details when a soft check rejects a precondition such as
isA<T>()and further expectations follow it, which also affectedevery:_fullDetailsassumed that when a subject itself failed, the failing clausewas the last one added to it. That holds for
check, where a failurethrows before anything else is added, but not for a soft check, where the
following nested clauses are still added, nor for an unawaited expectation,
which can fail after later clauses are added. Pass the rejected clause from
each failure through to
_fullDetailsinstead of relying on_clauses.last. A rejected guard is also no longer added a second time asa
_GuardClause, since the failing expectation clause already describesit, so the precondition appears once in the description.
Omit the clauses after the failing one from the failure details. They were
described under
Actual:as if the value satisfied them, and when thefailing clause came first the actual collapsed onto the root line with the
later clauses printed as its children.
Keep asynchronous expectations in the order they were made, and leave out
any which have not finished when another expectation fails. Nested
asynchronous expectations such as
completesandemitsadded their clauseonly once the value arrived, so an expectation made after them was described
before them. An expectation from
Context.expectAsyncadded its clause before its predicate finished, so itwas described as satisfied if a later expectation failed first. Both now
add a placeholder which keeps their place and is not described until they
finish. Expectations from
Context.expectUnawaited, such asdoesNotComplete, are never awaited by the check and are still describedas satisfied until they fail.
Make the
CheckFailureandFailureDetailconstructors private. Failuresonly come from soft checking a
Condition, sinceConditionandContextare
abstract final, and no API accepts a failure constructed outside thepackage. The public
FailureDetailconstructor also recorded that anycustom
actualstarted with the root label line, sodescribeNesteddropped its first line.