From 8ab184be2f26861f13be1d62f8acc635e65a9da6 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 3 Feb 2023 02:00:47 +0000 Subject: [PATCH] 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 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. --- pkgs/checks/lib/src/checks.dart | 48 ++++++------ pkgs/checks/lib/src/extensions/async.dart | 12 +-- pkgs/checks/lib/src/extensions/core.dart | 6 +- pkgs/checks/lib/src/extensions/function.dart | 4 +- pkgs/checks/lib/src/extensions/map.dart | 19 ++--- pkgs/checks/test/extensions/map_test.dart | 80 +++++++++++++++----- pkgs/checks/test/test_shared.dart | 9 ++- 7 files changed, 115 insertions(+), 63 deletions(-) diff --git a/pkgs/checks/lib/src/checks.dart b/pkgs/checks/lib/src/checks.dart index 2a09dea3a..e3f195eda 100644 --- a/pkgs/checks/lib/src/checks.dart +++ b/pkgs/checks/lib/src/checks.dart @@ -63,7 +63,7 @@ extension Skip on Subject { Subject check(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 { /// 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 nest(String label, Extracted Function(T) extract, + Subject nest( + Iterable Function() label, Extracted Function(T) extract, {bool atSameLevel = false}); /// Extract an asynchronous property from the value for further checking. @@ -277,8 +278,8 @@ abstract class Context { /// Some context may disallow asynchronous expectations, for instance in /// [softCheck] which must synchronously check the value. In those contexts /// this method will throw. - Future> nestAsync( - String label, FutureOr> Function(T) extract); + Future> nestAsync(Iterable Function() label, + FutureOr> Function(T) extract); } /// A property extracted from a value being checked, or a rejection. @@ -363,7 +364,7 @@ class _TestContext implements Context, _ClauseDescription { final List<_TestContext> _aliases; // The "a value" in "a value that:". - final String _label; + final Iterable Function() _label; final void Function(CheckFailure) _fail; @@ -375,9 +376,9 @@ class _TestContext implements Context, _ClauseDescription { required void Function(CheckFailure) fail, required bool allowAsync, required bool allowUnawaited, - String? label, + Iterable Function()? label, }) : _value = value, - _label = label ?? '', + _label = label ?? (() => ['']), _fail = fail, _allowAsync = allowAsync, _allowUnawaited = allowUnawaited, @@ -394,7 +395,7 @@ class _TestContext implements Context, _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 parent) : _parent = parent, @@ -444,12 +445,13 @@ class _TestContext implements Context, _ClauseDescription { } @override - Subject nest(String label, Extracted Function(T) extract, + Subject nest( + Iterable Function() label, Extracted 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(); @@ -457,7 +459,7 @@ class _TestContext implements Context, _ClauseDescription { 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 implements Context, _ClauseDescription { } @override - Future> nestAsync( - String label, FutureOr> Function(T) extract) async { + Future> nestAsync(Iterable Function() label, + FutureOr> Function(T) extract) async { if (!_allowAsync) { throw StateError( 'Async expectations cannot be used on a synchronous subject'); @@ -478,7 +480,7 @@ class _TestContext implements Context, _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(); @@ -507,9 +509,9 @@ class _TestContext implements Context, _ClauseDescription { var successfulOverlap = 0; final expected = []; 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 implements Context { } @override - Subject nest(String label, Extracted Function(T p1) extract, + Subject nest( + Iterable Function() label, Extracted Function(T p1) extract, {bool atSameLevel = false}) { return Subject._(_SkippedContext()); } @override - Future> nestAsync( - String label, FutureOr> Function(T p1) extract) async { + Future> nestAsync(Iterable Function() label, + FutureOr> Function(T p1) extract) async { return Subject._(_SkippedContext()); } } @@ -766,7 +769,8 @@ class _ReplayContext implements Context, Condition { } @override - Subject nest(String label, Extracted Function(T p1) extract, + Subject nest( + Iterable Function() label, Extracted Function(T p1) extract, {bool atSameLevel = false}) { final nestedContext = _ReplayContext(); _interactions.add((c) { @@ -777,8 +781,8 @@ class _ReplayContext implements Context, Condition { } @override - Future> nestAsync( - String label, FutureOr> Function(T) extract) async { + Future> nestAsync(Iterable Function() label, + FutureOr> Function(T) extract) async { final nestedContext = _ReplayContext(); _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 index 2633af3bf..bde4ba0fb 100644 --- a/pkgs/checks/lib/src/extensions/async.dart +++ b/pkgs/checks/lib/src/extensions/async.dart @@ -16,7 +16,7 @@ extension FutureChecks on Subject> { /// /// Fails if the future completes as an error. Future> completes() => - context.nestAsync('completes to a value', (actual) async { + context.nestAsync(() => ['completes to a value'], (actual) async { try { return Extracted.value(await actual); } catch (e, st) { @@ -61,7 +61,7 @@ extension FutureChecks on Subject> { /// /// Fails if the future completes to a value. Future> throws() => context.nestAsync( - '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 on Subject> { /// Fails if the stream emits an error instead of a value, or closes without /// emitting a value. Future> emits() => - context.nestAsync('emits a value', (actual) async { + context.nestAsync(() => ['emits a value'], (actual) async { if (!await actual.hasNext) { return Extracted.rejection( actual: ['a stream'], @@ -140,8 +140,8 @@ extension StreamChecks on Subject> { /// If this expectation fails, the source queue will be left in it's original /// state. /// If this expectation succeeds, consumes the error event. - Future> emitsError() => - context.nestAsync('emits an error${E == Object ? '' : ' of type $E'}', + Future> emitsError() => 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 on Subject> { /// so that they can support conditional expectations and check multiple /// possibilities from the same point in the stream. Subject> 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 index 1d32b8894..38ba4c76a 100644 --- a/pkgs/checks/lib/src/extensions/core.dart +++ b/pkgs/checks/lib/src/extensions/core.dart @@ -10,7 +10,7 @@ extension CoreChecks on Subject { /// Sets up a clause that the value "has [name] that:" followed by any /// expectations applied to the returned [Subject]. Subject has(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 on Subject { /// /// If the value is a [T], returns a [Subject] for further expectations. Subject isA() { - return context.nest('is a $R', (actual) { + return context.nest(() => ['is a $R'], (actual) { if (actual is! R) { return Extracted.rejection(which: ['Is a ${actual.runtimeType}']); } @@ -118,7 +118,7 @@ extension BoolChecks on Subject { extension NullabilityChecks on Subject { Subject isNotNull() { - return context.nest('is not null', (actual) { + return context.nest(() => ['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 index 7e20aca61..1caa2aeb4 100644 --- a/pkgs/checks/lib/src/extensions/function.dart +++ b/pkgs/checks/lib/src/extensions/function.dart @@ -17,7 +17,7 @@ extension ThrowsCheck on Subject { /// fail. Instead invoke the function and check the expectation on the /// returned [Future]. Subject throws() { - return context.nest('throws an error of type $E', (actual) { + return context.nest(() => ['throws an error of type $E'], (actual) { try { final result = actual(); return Extracted.rejection( @@ -40,7 +40,7 @@ extension ThrowsCheck on Subject { /// /// If the function throws synchronously, this expectation will fail. Subject returnsNormally() { - return context.nest('returns a value', (actual) { + return context.nest(() => ['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 index b836f1ee5..4984b5fb2 100644 --- a/pkgs/checks/lib/src/extensions/map.dart +++ b/pkgs/checks/lib/src/extensions/map.dart @@ -14,11 +14,11 @@ extension MapChecks on Subject> { Subject> get values => has((m) => m.values, 'values'); Subject get length => has((m) => m.length, 'length'); Subject 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 on Subject> { /// 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 on Subject> { /// 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 index 8cbee0e06..8eb0eb291 100644 --- 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>()..['some\nlong\nkey']) + .description + .deepEquals([ + " contains a value for 'some", + ' long', + " key'", + ]); + check(it>()..['some\nlong\nkey'].equals(1)) + .description + .deepEquals([ + " contains a value for 'some", + ' long', + " key' that:", + ' equals <1>', + ]); + }); }); test('isEmpty', () { check({}).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>()..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>()..containsValue('some\nlong\nkey')) + .description + .deepEquals([ + " contains value 'some", + ' long', + " key'", + ]); + }); }); test('containsValueThat', () { check(_testMap).containsValueThat(it()..equals(1)); diff --git a/pkgs/checks/test/test_shared.dart b/pkgs/checks/test/test_shared.dart index 325c63279..1a30eb55a 100644 --- a/pkgs/checks/test/test_shared.dart +++ b/pkgs/checks/test/test_shared.dart @@ -10,8 +10,8 @@ extension RejectionChecks on Subject { {Iterable? actual, Iterable? which}) { late T actualValue; var didRunCallback = false; - final rejection = context - .nest('does not meet a condition with a Rejection', (value) { + final rejection = context.nest( + () => ['does not meet a condition with a Rejection'], (value) { actualValue = value; didRunCallback = true; final failure = softCheck(value, condition); @@ -45,7 +45,8 @@ extension RejectionChecks on Subject { late T actualValue; var didRunCallback = false; final rejection = (await context.nestAsync( - 'does not meet an async condition with a Rejection', (value) async { + () => ['does not meet an async condition with a Rejection'], + (value) async { actualValue = value; didRunCallback = true; final failure = await softCheckAsync(value, condition); @@ -80,7 +81,7 @@ extension ConditionChecks on Subject> { has((c) => describe(c), 'description'); Future>> get asyncDescription async => context.nestAsync( - 'has description', + () => ['has description'], (condition) async => Extracted.value(await describeAsync(condition))); }