Restore Condition as a class by natebosch · Pull Request #2697 · dart-lang/test

natebosch · 2026-07-17T01:40:57Z

Reintroduce Condition<T> as a class in place of callback typedefs
(void Function(Subject<T>) / Future<void> Function(Subject<T>)).

Previously, the Condition class was dropped in favor of callback
functions, in part to avoid it() appearing like a language feature.
However, using callback functions introduced type inference limitations.

Callbacks required declaring explicit parameters, introducing
unnecessary boilerplate across expectation calls.
When explicit types are required it takes a full Subject<SomeType>
as the argument type annotation, mixing the details of the
implementation with the goal of passing a type argument.
When type inference failed (such as in collection matching or
heterogeneous checks), callbacks resulted in cryptic runtime errors
regarding missing extension methods on Subject with no static
assistance.

Reintroducing Condition<T> with Condition.it<T>() static helper.

Dot shorthands provide a terse, readable syntax
(.it()..isGreaterThan(0)) without polluting the top-level namespace.
When inference fails Condition.it<SomeType>() is more explicit and
readable than (Subject<SomeType> it) =>.
Consolidating softCheck, softCheckSync, describe, and
describeSync as instance methods on Condition<T> (and eliminating
standalone top-level functions softCheck, softCheckAsync,
describe, describeAsync) improves overall API discoverability and
ergonomics, despite being a breaking change.

Example Migration:

Before:

check(numbers).any((it) => it.isGreaterThan(5));

check(map).containsKeyThat((key) => key
  ..startsWith('a')
  ..has((s) => s.length, 'length').isGreaterThan(2));

After:

check(numbers).any(.it()..isGreaterThan(5));

check(map).containsKeyThat(.it()
  ..startsWith('a')
  ..has((s) => s.length, 'length').isGreaterThan(2));

Closes #2694 Reintroduce `Condition<T>` as a class in place of callback typedefs (`void Function(Subject<T>)` / `Future<void> Function(Subject<T>)`). Previously, the `Condition` class was dropped in favor of callback functions, in part to avoid `it()` appearing like a language feature. However, using callback functions introduced type inference limitations. - Callbacks required declaring explicit parameters (e.g., `(it) => ...`), introducing unnecessary boilerplate across expectation calls. - When explicit types are required it takes a full `Subject<SomeType>` as the argument type annotation, mixing the details of the implementation with the goal of passing a type argument. - When type inference failed (such as in collection matching or heterogeneous checks), callbacks resulted in cryptic runtime errors regarding missing extension methods on `Subject` with no static assistance. Reintroducing `Condition<T>` with `Condition.it<T>()` static helper. - Dot shorthands provide a terse, readable syntax (`.it()..isGreaterThan(0)`) without polluting the top-level namespace. - When inference fails `Condition.it<SomeType>()` is more explicit and readable than `(Subject<SomeType> it) =>`. - Consolidating `softCheck`, `softCheckSync`, `describe`, and `describeSync` as instance methods on `Condition<T>` (and eliminating standalone top-level functions `softCheck`, `softCheckAsync`, `describe`, `describeAsync`) improves overall API discoverability and ergonomics, despite being a breaking change. Example Migration: Before: ```dart check(numbers).any((it) => it.isGreaterThan(5)); check(map).containsKeyThat((key) => key ..startsWith('a') ..has((s) => s.length, 'length').isGreaterThan(2)); ``` After: ```dart check(numbers).any(.it()..isGreaterThan(5)); check(map).containsKeyThat(.it() ..startsWith('a') ..has((s) => s.length, 'length').isGreaterThan(2)); ```

natebosch · 2026-07-17T22:59:52Z

You can see the impact on test code in the diff here - somewhat eaiser if you start at async_test.dart, expand it, and read down from there.

Probably even easier to see the impact on tests in package:dart_mcp:

example of migrating tests that can't use dot shorthands yet: dart-lang/ai@eb15b49
If we were on a later language version: dart-lang/ai@16680e1

This PR sits on top of #2683 because it's a more drastic change when there are more extensions using this syntax. We can land this without the other easily enough if we decide to. If we decide not to land this one I think I'd be less inclined to land the other.

lrhn · 2026-07-20T08:15:03Z

Probably could do something monadic like:

check(numbers).any.isGreaterThan(5);

but it's more complexity, and monads may not always nest nicely.

I like this approach. It generalizes abstracting over operations on a Subject<T>.
It does so by recording the operations on the context, so it can replay them on a another context.
That's neat, any syntactic sugar on top of the basic operations will just work.

I'd love to take it a bit further ... but that's not realistic.
Take for example:

check(numbers).any(.it()..isGreaterThan(5));

If we could just make any return a ConditionSubject, so you could write:

check(numbers).any.isGreaterThan(5);

That sadly has serious timing issues, the any call has lost control before the condition
has been filled in. It doesn't know when it can start checking.
Would require serious hackery to get around that, and probably remove any benefit.
(That's where having a specific list-monad-subject would work, because it would get the
calls directly, not indirectly from a Condition.)

lrhn · 2026-07-20T08:31:05Z

+          }
+        },
+        errorCondition,
+      );


(I had an idea, so I'll put it here 😁:
What if there was Future<Subject<E>> ifThrows<E extends Object>([Condition<E>? condition]) which:

If the future throws an E, it's checked againsts condition, and if successful, returned.

If it throws a non-E, throws an E not satisfying condition, or it completes with a value, the returned subject is a "skipped" subject that can't fail.

So more of a guard than a check.

And similar for ifCompletes

Then you could do

check(parseFuture) ..ifCompletes(.it.equals(42)) ..ifThrows(.it.isA<FormatException>());

Have to make sure all error futures are properly handled on all branches.)

lrhn · 2026-07-20T08:33:54Z

-///
-/// Any expectations may be used within an `AsyncCondition` callback.
-typedef AsyncCondition<T> = FutureOr<void> Function(Subject<T>);
-


(Thanks. I hate typedefs in APIs. I look at a signature in an IDE and it just says AsyncCondition<T>, and then I still don't know what type I have to write. I need to go look at the declaration anyway.)

lrhn · 2026-07-20T08:35:29Z

+/// non-[Future] values always, but will throw an [AsyncConditionDisallowed]
+/// exception if asynchronous expectations were invoked on the condition.
+abstract final class Condition<T> {
+  static ConditionSubject<T> it<T>() => ._();


(It's really annoying that this has to be a method, just because it has to be generic. We should have generic getters!)

lrhn · 2026-07-20T08:58:53Z

-  await condition(subject);
-  return failure;
-}
+  /// Run these exepctations against [subject].


Typo -> expectations

lrhn · 2026-07-20T09:00:41Z

+  /// otherwise it will complete to the [CheckFailure] for the first expectation
+  /// that fails.
+  ///
+  /// Asynchronous expectations are not allowed.


(Is this a failure represented by a check failure or will that throw an exception? In which case the first line is slighlty misleading)

lrhn · 2026-07-20T09:01:52Z

+  /// Run these expectations against [subject].
+  ///
+  /// Asynchronous expectations are not allowed and will cause a runtime error
+  /// if they were used.


(Use this paragraph for the other *Sync functions too, it's more explicit than "is not allowed".)

lrhn · 2026-07-20T09:26:15Z

  /// Fails if the future completes to a value.
  ///
  /// Pass [errorCondition] to check expectations on the error thrown by the
  /// future.


Is [ErrorCondition] needed.

Is (await subject.throws<E>()).tests equivalent to await subject.throws<E>(it()..tests)?

If you can do both, which one should we recommend?
(The former shoud work with expectSync, the latter ... might?)

lrhn · 2026-07-20T09:39:45Z

  /// future.
  ///
  /// The returned future will complete when the subject future has completed,
  /// and [errorCondition] has optionally been checked.


This errorCondition is check-functions called on the Subject<E> that is caught.

I'm not sure it matters whether the condition is used as a real check, or only as a predicate, other than whether the error of

await check(Future.error(StateError('Apricot')) .throws(.it().message.equals('Bananas'));

Is

Did not throw a StateError that has a message which equals "Bananas".
or
Threw a StateError which has a message which does not equal "Bananas".

That is, whether the test is "did it throw E + do checks on E" or "did it throw E which satisfies predicate".

I think it should be the latter.
(Also because the former is equivalent to (await subject.throws<E>()).checks() anyway, so you can get that if you want it.)

Also, some tests promote, like isA.
Someone might question why:

check(future).completes<Object?>(.it().isA<int>())

does not return Future<Subject<int>>.
(There are good reasons, but "it's only used as a predicate" is an easier explanation.)

lrhn · 2026-07-20T09:44:58Z

+          return ['emits any values then emits a value that:', ...description];
+        }
+        return [
+          'emits any values then emits a value satisfying an asynchronous condition',


(Reading this, I didn't read "any values" as allowing no values. The name didn't help me - I have no intuition what "emits through" means.
I guess it's the inclusive version of emitsUntil, and it's on a StreamQueue.
Consider emitsEventual or emitsAny, and just document that it accepts any events until such one.)

lrhn · 2026-07-20T09:46:56Z

  ///
  /// Waits for each condition to be satisfied or rejected before checking the
  /// next. Subsequent conditions will not see any events consumed by earlier
  /// conditions.


(Can maybe say something like "one event per condition, in order" early. I had to read to here to figure out whether one event can satsify more than one condition.
Also "consumed" is not a word used earlier.)

lrhn · 2026-07-20T09:47:48Z

-          }),
-        );
+        final descriptionFuture = conditions
+            .map((c) async => await c.describe())


(That's just (c) => c.describe() with more words, and maybe more delays.)

lrhn · 2026-07-20T09:49:34Z

-        );
+        final descriptionFuture = conditions
+            .map((c) async => await c.describe())
+            .wait;


Can .describe() throw?
(If not, is it documented on the function that it must not throw?)

If it can, then this future may complete with an unhandled async error while doing the next statement.

(If it can't, it'd be nice to know.)

lrhn · 2026-07-20T09:52:46Z

+            longestAccepted = copy;
+          }
+          return failure;
+        }).wait;


If any of the futures in failures can end in an error, which they can if any of the softChecks throw, and those can be synchronous and calls user code so they almost certainly can throw, ... then the failures future may complete with an unhandled asynchronous error while await descriptionFuture runs below.

This is just a test, and you probably know it won't throw, so document that.

lrhn · 2026-07-20T09:53:13Z

+          if (failure == null &&
+              (longestAccepted == null ||
+                  copy.eventsDispatched > longestAccepted!.eventsDispatched)) {
+            longestAccepted = copy;


Should the shorter of longestAccepted and copy be closed in some way?

lrhn · 2026-07-20T09:59:08Z

+        }).wait;
+
        descriptions.addAll(await descriptionFuture);
        if (longestAccepted != null) {


case final accepted? for the promotion.

lrhn · 2026-07-20T10:08:50Z

+/// instances are run against the value in the same position within [actual].
+/// Condition instances must be a `Condition<Object?>` or
+/// `Condition<dynamic>` and may not use a more specific generic.
+/// Use `Condition.it<Object?>()..isA<Type>()` to check expectations for


Should it be .isA<Type>() and not ..isA<Type>() here? (So you can use checks on Types on it.)

lrhn · 2026-07-20T10:34:00Z

  /// ```
  /// check(something)
  ///   ..has((s) => s.foo, 'foo').equals(expectedFoo)
  ///   ..has((s) => s.bar, 'bar').which((b) => b


Old format.

which(it()..isLessThan(10)..isGreaterThan(0));

lrhn · 2026-07-20T10:36:36Z

  ///     ..isGreaterThan(0));
  /// ```
-  void which(Condition<T> condition) => condition(this);
+  void which(Condition<T> condition) => condition.applySync(this);


This change may make which more important.
It introduces Condition, an application of checks, as a first-class value that you may want to store for reuse.
You can have whole libraries of conditions. (Probably shouldn't, those should be extensions on subjects of their types instead.)
But still people may want to use which more.

Do we need both which and isA?

What if isA was

/// Checks that the valie is an [R] which satisfies [condition]. R isA<R extends T>([Condition<R>? condition]);

(Only allowing downcasts to make .isA(.it()...) work better, maybe have castsTo<T> for general casts.
And because also having an operation that only allows downcasts is better than only having one that allows anything without warning that it's unrelated.)

lrhn · 2026-07-20T10:49:52Z

-  return context.detail(context).expected.skip(1);
+/// A [Subject] that records the checks that are performed and expose them as an
+/// [Condition].
+final class ConditionSubject<T> implements Condition<T>, Subject<T> {


ConditionSubject has public methods, which means that you can't use those names as the first check on a condition.

check(myFunctionAbstraction).where(.it()..apply().equals(42));

where apply is a custom extension on Subject<MyFunctionAbstraction>, won't work,
but if it was .it()..isA<MyFunctionAbstraction>().apply().equals(42), it would because the static type of isA is just a Subject.

Can keep the methods public, but change it() to return Subject<T> instead of ConditionSubject<T>.
(Does the class and methods need to be public?)

lrhn

I like the approach. I tries to sidestep the "monadic subject" (like AsyncSubject) by abstracting over the things you'd do those subjects, and then injecting them into the Subject<Monad<T>>.
That way there doesn't have to be special subjects for each kind of weird context a value can be in. (And it generalizes.)

It shouldn't be more or less powerful than FutureOr<void> Function(Subject<T> subject), but it's probably easier to work with for users. (And may be easier to type-infer.)

Maybe the Condition class, and the it method, should state very directly that you should always us .it().. to ensure the result is a Condition, since the check calls on it return a normal Subject.

natebosch requested a review from a team as a code owner July 17, 2026 01:40

github-actions Bot added the package:checks Issues related to pkg:checks label Jul 17, 2026

natebosch marked this pull request as draft July 17, 2026 01:42

Update uses

7ef2c11

natebosch mentioned this pull request Jul 17, 2026

Change expectations which take no arguments to be getters - check(foo).isTrue instead of check(foo).isTrue() #1943

Open

natebosch mentioned this pull request Jul 17, 2026

Allow similar use between sync and async checks #2683

Open

Merge branch 'sync-condition-callbacks' into condition-as-class-2

2496a09

natebosch mentioned this pull request Jul 18, 2026

Add a top level equals utility #1897

Closed

lrhn reviewed Jul 20, 2026

View reviewed changes

lrhn approved these changes Jul 20, 2026

View reviewed changes

Uh oh!

Conversation

natebosch commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

natebosch commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lrhn commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lrhn Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrhn Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrhn Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrhn Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrhn Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrhn left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

natebosch commented Jul 17, 2026 •

edited

Loading

natebosch commented Jul 17, 2026 •

edited

Loading

lrhn commented Jul 20, 2026 •

edited

Loading

lrhn Jul 20, 2026 •

edited

Loading

lrhn Jul 20, 2026 •

edited

Loading

lrhn Jul 20, 2026 •

edited

Loading

lrhn Jul 20, 2026 •

edited

Loading

lrhn Jul 20, 2026 •

edited

Loading