Make label arguments take callbacks

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.
dart-lang · Feb 3, 2023 · 8ab184b · 8ab184b
1 parent d2858ba
commit 8ab184b
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 63 deletions.
diff --git a/pkgs/checks/lib/src/checks.dart b/pkgs/checks/lib/src/checks.dart
@@ -63,7 +63,7 @@ extension Skip<T> on Subject<T> {
 Subject<T> check<T>(T value, {String? because}) => Subject._(_TestContext._root(
       value: _Present(value),
       // TODO - switch between "a" and "an"
-      label: 'a $T',
+      label: () => ['a $T'],
       fail: (f) {
         final which = f.rejection.which;
         throw TestFailure([
@@ -261,7 +261,8 @@ abstract class Context<T> {
   /// context. The [label] will be used as if it were a single line "clause"
   /// passed to [expect]. If the label is empty, the clause will be omitted. The
   /// label should only be left empty if the value extraction cannot fail.
-  Subject<R> nest<R>(String label, Extracted<R> Function(T) extract,
+  Subject<R> nest<R>(
+      Iterable<String> Function() label, Extracted<R> Function(T) extract,
       {bool atSameLevel = false});
 
   /// Extract an asynchronous property from the value for further checking.
@@ -277,8 +278,8 @@ abstract class Context<T> {
   /// Some context may disallow asynchronous expectations, for instance in
   /// [softCheck] which must synchronously check the value. In those contexts
   /// this method will throw.
-  Future<Subject<R>> nestAsync<R>(
-      String label, FutureOr<Extracted<R>> Function(T) extract);
+  Future<Subject<R>> nestAsync<R>(Iterable<String> Function() label,
+      FutureOr<Extracted<R>> Function(T) extract);
 }
 
 /// A property extracted from a value being checked, or a rejection.
@@ -363,7 +364,7 @@ class _TestContext<T> implements Context<T>, _ClauseDescription {
   final List<_TestContext> _aliases;
 
   // The "a value" in "a value that:".
-  final String _label;
+  final Iterable<String> Function() _label;
 
   final void Function(CheckFailure) _fail;
 
@@ -375,9 +376,9 @@ class _TestContext<T> implements Context<T>, _ClauseDescription {
     required void Function(CheckFailure) fail,
     required bool allowAsync,
     required bool allowUnawaited,
-    String? label,
+    Iterable<String> Function()? label,
   })  : _value = value,
-        _label = label ?? '',
+        _label = label ?? (() => ['']),
         _fail = fail,
         _allowAsync = allowAsync,
         _allowUnawaited = allowUnawaited,
@@ -394,7 +395,7 @@ class _TestContext<T> implements Context<T>, _ClauseDescription {
         _allowUnawaited = original._allowUnawaited,
         // Never read from an aliased context because they are never present in
         // `_clauses`.
-        _label = '';
+        _label = (() => ['']);
 
   _TestContext._child(this._value, this._label, _TestContext<dynamic> parent)
       : _parent = parent,
@@ -444,20 +445,21 @@ class _TestContext<T> implements Context<T>, _ClauseDescription {
   }
 
   @override
-  Subject<R> nest<R>(String label, Extracted<R> Function(T) extract,
+  Subject<R> nest<R>(
+      Iterable<String> Function() label, Extracted<R> Function(T) extract,
       {bool atSameLevel = false}) {
     final result = _value.map((actual) => extract(actual)._fillActual(actual));
     final rejection = result.rejection;
     if (rejection != null) {
-      _clauses.add(_StringClause(() => [label]));
+      _clauses.add(_StringClause(label));
       _fail(_failure(rejection));
     }
     final value = result.value ?? _Absent<R>();
     final _TestContext<R> context;
     if (atSameLevel) {
       context = _TestContext._alias(this, value);
       _aliases.add(context);
-      if (label.isNotEmpty) _clauses.add(_StringClause(() => [label]));
+      _clauses.add(_StringClause(label));
     } else {
       context = _TestContext._child(value, label, this);
       _clauses.add(context);
@@ -466,8 +468,8 @@ class _TestContext<T> implements Context<T>, _ClauseDescription {
   }
 
   @override
-  Future<Subject<R>> nestAsync<R>(
-      String label, FutureOr<Extracted<R>> Function(T) extract) async {
+  Future<Subject<R>> nestAsync<R>(Iterable<String> Function() label,
+      FutureOr<Extracted<R>> Function(T) extract) async {
     if (!_allowAsync) {
       throw StateError(
           'Async expectations cannot be used on a synchronous subject');
@@ -478,7 +480,7 @@ class _TestContext<T> implements Context<T>, _ClauseDescription {
     outstandingWork.complete();
     final rejection = result.rejection;
     if (rejection != null) {
-      _clauses.add(_StringClause(() => [label]));
+      _clauses.add(_StringClause(label));
       _fail(_failure(rejection));
     }
     final value = result.value ?? _Absent<R>();
@@ -507,9 +509,9 @@ class _TestContext<T> implements Context<T>, _ClauseDescription {
     var successfulOverlap = 0;
     final expected = <String>[];
     if (_clauses.isEmpty) {
-      expected.add(_label);
+      expected.addAll(_label());
     } else {
-      expected.add('$_label that:');
+      expected.addAll(postfixLast(' that:', _label()));
       for (var clause in _clauses) {
         final details = clause.detail(failingContext);
         expected.addAll(indent(details.expected));
@@ -550,14 +552,15 @@ class _SkippedContext<T> implements Context<T> {
   }
 
   @override
-  Subject<R> nest<R>(String label, Extracted<R> Function(T p1) extract,
+  Subject<R> nest<R>(
+      Iterable<String> Function() label, Extracted<R> Function(T p1) extract,
       {bool atSameLevel = false}) {
     return Subject._(_SkippedContext());
   }
 
   @override
-  Future<Subject<R>> nestAsync<R>(
-      String label, FutureOr<Extracted<R>> Function(T p1) extract) async {
+  Future<Subject<R>> nestAsync<R>(Iterable<String> Function() label,
+      FutureOr<Extracted<R>> Function(T p1) extract) async {
     return Subject._(_SkippedContext());
   }
 }
@@ -766,7 +769,8 @@ class _ReplayContext<T> implements Context<T>, Condition<T> {
   }
 
   @override
-  Subject<R> nest<R>(String label, Extracted<R> Function(T p1) extract,
+  Subject<R> nest<R>(
+      Iterable<String> Function() label, Extracted<R> Function(T p1) extract,
       {bool atSameLevel = false}) {
     final nestedContext = _ReplayContext<R>();
     _interactions.add((c) {
@@ -777,8 +781,8 @@ class _ReplayContext<T> implements Context<T>, Condition<T> {
   }
 
   @override
-  Future<Subject<R>> nestAsync<R>(
-      String label, FutureOr<Extracted<R>> Function(T) extract) async {
+  Future<Subject<R>> nestAsync<R>(Iterable<String> Function() label,
+      FutureOr<Extracted<R>> Function(T) extract) async {
     final nestedContext = _ReplayContext<R>();
     _interactions.add((c) async {
       var result = await c.nestAsync(label, extract);

diff --git a/pkgs/checks/lib/src/extensions/async.dart b/pkgs/checks/lib/src/extensions/async.dart
@@ -16,7 +16,7 @@ extension FutureChecks<T> on Subject<Future<T>> {
   ///
   /// Fails if the future completes as an error.
   Future<Subject<T>> completes() =>
-      context.nestAsync<T>('completes to a value', (actual) async {
+      context.nestAsync<T>(() => ['completes to a value'], (actual) async {
         try {
           return Extracted.value(await actual);
         } catch (e, st) {
@@ -61,7 +61,7 @@ extension FutureChecks<T> on Subject<Future<T>> {
   ///
   /// Fails if the future completes to a value.
   Future<Subject<E>> throws<E extends Object>() => context.nestAsync<E>(
-          'completes to an error${E == Object ? '' : ' of type $E'}',
+          () => ['completes to an error${E == Object ? '' : ' of type $E'}'],
           (actual) async {
         try {
           return Extracted.rejection(
@@ -110,7 +110,7 @@ extension StreamChecks<T> on Subject<StreamQueue<T>> {
   /// Fails if the stream emits an error instead of a value, or closes without
   /// emitting a value.
   Future<Subject<T>> emits() =>
-      context.nestAsync<T>('emits a value', (actual) async {
+      context.nestAsync<T>(() => ['emits a value'], (actual) async {
         if (!await actual.hasNext) {
           return Extracted.rejection(
               actual: ['a stream'],
@@ -140,8 +140,8 @@ extension StreamChecks<T> on Subject<StreamQueue<T>> {
   /// If this expectation fails, the source queue will be left in it's original
   /// state.
   /// If this expectation succeeds, consumes the error event.
-  Future<Subject<E>> emitsError<E extends Object>() =>
-      context.nestAsync('emits an error${E == Object ? '' : ' of type $E'}',
+  Future<Subject<E>> emitsError<E extends Object>() => context.nestAsync(
+          () => ['emits an error${E == Object ? '' : ' of type $E'}'],
           (actual) async {
         if (!await actual.hasNext) {
           return Extracted.rejection(
@@ -462,6 +462,6 @@ extension StreamQueueWrap<T> on Subject<Stream<T>> {
   /// so that they can support conditional expectations and check multiple
   /// possibilities from the same point in the stream.
   Subject<StreamQueue<T>> get withQueue =>
-      context.nest('', (actual) => Extracted.value(StreamQueue(actual)),
+      context.nest(() => [], (actual) => Extracted.value(StreamQueue(actual)),
           atSameLevel: true);
 }
diff --git a/pkgs/checks/lib/src/extensions/core.dart b/pkgs/checks/lib/src/extensions/core.dart
@@ -10,7 +10,7 @@ extension CoreChecks<T> on Subject<T> {
   /// Sets up a clause that the value "has [name] that:" followed by any
   /// expectations applied to the returned [Subject].
   Subject<R> has<R>(R Function(T) extract, String name) {
-    return context.nest('has $name', (T value) {
+    return context.nest(() => ['has $name'], (T value) {
       try {
         return Extracted.value(extract(value));
       } catch (_) {
@@ -70,7 +70,7 @@ extension CoreChecks<T> on Subject<T> {
   ///
   /// If the value is a [T], returns a [Subject] for further expectations.
   Subject<R> isA<R>() {
-    return context.nest<R>('is a $R', (actual) {
+    return context.nest<R>(() => ['is a $R'], (actual) {
       if (actual is! R) {
         return Extracted.rejection(which: ['Is a ${actual.runtimeType}']);
       }
@@ -118,7 +118,7 @@ extension BoolChecks on Subject<bool> {
 
 extension NullabilityChecks<T> on Subject<T?> {
   Subject<T> isNotNull() {
-    return context.nest<T>('is not null', (actual) {
+    return context.nest<T>(() => ['is not null'], (actual) {
       if (actual == null) return Extracted.rejection();
       return Extracted.value(actual);
     }, atSameLevel: true);

diff --git a/pkgs/checks/lib/src/extensions/function.dart b/pkgs/checks/lib/src/extensions/function.dart
@@ -17,7 +17,7 @@ extension ThrowsCheck<T> on Subject<T Function()> {
   /// fail. Instead invoke the function and check the expectation on the
   /// returned [Future].
   Subject<E> throws<E>() {
-    return context.nest<E>('throws an error of type $E', (actual) {
+    return context.nest<E>(() => ['throws an error of type $E'], (actual) {
       try {
         final result = actual();
         return Extracted.rejection(
@@ -40,7 +40,7 @@ extension ThrowsCheck<T> on Subject<T Function()> {
   ///
   /// If the function throws synchronously, this expectation will fail.
   Subject<T> returnsNormally() {
-    return context.nest<T>('returns a value', (actual) {
+    return context.nest<T>(() => ['returns a value'], (actual) {
       try {
         return Extracted.value(actual());
       } catch (e, st) {

diff --git a/pkgs/checks/lib/src/extensions/map.dart b/pkgs/checks/lib/src/extensions/map.dart
@@ -14,11 +14,11 @@ extension MapChecks<K, V> on Subject<Map<K, V>> {
   Subject<Iterable<V>> get values => has((m) => m.values, 'values');
   Subject<int> get length => has((m) => m.length, 'length');
   Subject<V> operator [](K key) {
-    final keyString = literal(key).join(r'\n');
-    return context.nest('contains a value for $keyString', (actual) {
+    return context.nest(
+        () => prefixFirst('contains a value for ', literal(key)), (actual) {
       if (!actual.containsKey(key)) {
         return Extracted.rejection(
-            which: ['does not contain the key $keyString']);
+            which: prefixFirst('does not contain the key ', literal(key)));
       }
       return Extracted.value(actual[key] as V);
     });
@@ -40,10 +40,10 @@ extension MapChecks<K, V> on Subject<Map<K, V>> {
 
   /// Expects that the map contains [key] according to [Map.containsKey].
   void containsKey(K key) {
-    final keyString = literal(key).join(r'\n');
-    context.expect(() => ['contains key $keyString'], (actual) {
+    context.expect(() => prefixFirst('contains key ', literal(key)), (actual) {
       if (actual.containsKey(key)) return null;
-      return Rejection(which: ['does not contain key $keyString']);
+      return Rejection(
+          which: prefixFirst('does not contain key ', literal(key)));
     });
   }
 
@@ -68,10 +68,11 @@ extension MapChecks<K, V> on Subject<Map<K, V>> {
 
   /// Expects that the map contains [value] according to [Map.containsValue].
   void containsValue(V value) {
-    final valueString = literal(value).join(r'\n');
-    context.expect(() => ['contains value $valueString'], (actual) {
+    context.expect(() => prefixFirst('contains value ', literal(value)),
+        (actual) {
       if (actual.containsValue(value)) return null;
-      return Rejection(which: ['does not contain value $valueString']);
+      return Rejection(
+          which: prefixFirst('does not contain value ', literal(value)));
     });
   }
 

diff --git a/pkgs/checks/test/extensions/map_test.dart b/pkgs/checks/test/extensions/map_test.dart
@@ -30,10 +30,31 @@ void main() {
     check(_testMap).values.contains(1);
   });
 
-  test('operator []', () async {
-    check(_testMap)['a'].equals(1);
-    check(_testMap)
-        .isRejectedBy(it()..['z'], which: ['does not contain the key \'z\'']);
+  group('operator []', () {
+    test('succeeds for a key that exists', () {
+      check(_testMap)['a'].equals(1);
+    });
+    test('fails for a missing key', () {
+      check(_testMap)
+          .isRejectedBy(it()..['z'], which: ['does not contain the key \'z\'']);
+    });
+    test('can be described', () {
+      check(it<Map<String, Object>>()..['some\nlong\nkey'])
+          .description
+          .deepEquals([
+        "  contains a value for 'some",
+        '  long',
+        "  key'",
+      ]);
+      check(it<Map<String, Object>>()..['some\nlong\nkey'].equals(1))
+          .description
+          .deepEquals([
+        "  contains a value for 'some",
+        '  long',
+        "  key' that:",
+        '    equals <1>',
+      ]);
+    });
   });
   test('isEmpty', () {
     check(<String, int>{}).isEmpty();
@@ -43,13 +64,25 @@ void main() {
     check(_testMap).isNotEmpty();
     check({}).isRejectedBy(it()..isNotEmpty(), which: ['is not empty']);
   });
-  test('containsKey', () {
-    check(_testMap).containsKey('a');
-
-    check(_testMap).isRejectedBy(
-      it()..containsKey('c'),
-      which: ["does not contain key 'c'"],
-    );
+  group('containsKey', () {
+    test('succeeds for a key that exists', () {
+      check(_testMap).containsKey('a');
+    });
+    test('fails for a missing key', () {
+      check(_testMap).isRejectedBy(
+        it()..containsKey('c'),
+        which: ["does not contain key 'c'"],
+      );
+    });
+    test('can be described', () {
+      check(it<Map<String, Object>>()..containsKey('some\nlong\nkey'))
+          .description
+          .deepEquals([
+        "  contains key 'some",
+        '  long',
+        "  key'",
+      ]);
+    });
   });
   test('containsKeyThat', () {
     check(_testMap).containsKeyThat(it()..equals('a'));
@@ -58,12 +91,25 @@ void main() {
       which: ['Contains no matching key'],
     );
   });
-  test('containsValue', () {
-    check(_testMap).containsValue(1);
-    check(_testMap).isRejectedBy(
-      it()..containsValue(3),
-      which: ['does not contain value <3>'],
-    );
+  group('containsValue', () {
+    test('succeeds for happy case', () {
+      check(_testMap).containsValue(1);
+    });
+    test('fails for missing value', () {
+      check(_testMap).isRejectedBy(
+        it()..containsValue(3),
+        which: ['does not contain value <3>'],
+      );
+    });
+    test('can be described', () {
+      check(it<Map<String, String>>()..containsValue('some\nlong\nkey'))
+          .description
+          .deepEquals([
+        "  contains value 'some",
+        '  long',
+        "  key'",
+      ]);
+    });
   });
   test('containsValueThat', () {
     check(_testMap).containsValueThat(it()..equals(1));